Page MenuHome

Stipple support to basic GLSL shader
ClosedPublic

Authored by Alexander Romanov (a.romanov) on Dec 24 2015, 2:52 PM.

Details

Summary

The patch is intended to replace deprecated glPolygonStipple() calls with shader based alternative.
(BGE is not touched yet, not all replaced code is tested, but you can review the code structure)

Alexander (Blend4Web Team)

Diff Detail

Repository
rB Blender

Event Timeline

Thanks! The overall approach seems good to me, it follows what we discussed. I have a few comments.

source/blender/blenkernel/intern/cdderivedmesh.c
769–772

This should use GPU_basic_shader_bind(GPU_SHADER_STIPPLE | GPU_SHADER_USE_COLOR);, since draw_dm_faces_sel__setDrawOptions calls glColor before returning DM_DRAW_OPTION_STIPPLE.

It's best to remove DM_DRAW_OPTION_STIPPLE entirely actually and the stippling shaders should be bound in draw_dm_faces_sel__setDrawOptions, and the derivedmeshes should be decoupled from shaders better. I can do to that as a next step after this is committed.

source/blender/editors/interface/interface_draw.c
1137–1143

We could add a comment here saying that this could be optimized with a single checkerboard shader, instead of drawing twice and using stippling the second time.

source/blender/gpu/intern/gpu_basic_shader.c
251

I get this build warning:

gpu_basic_shader.c:251:2: warning: excess elements in array initializer
        0xaa, 0xaa, 0xaa, 0xaa, 0x55, 0x55, 0x55, 0x55,
        ^~~~
source/blender/gpu/shaders/gpu_shader_basic_frag.glsl
56

A bit unfortunate that we have to use mod and integer casting when these are very simple bitwise operations. Could you add a comment saying that this can be optimized further when GLSL 1.3 is supported?

source/blender/windowmanager/intern/wm_stereo.c
93

swap seems to not be taken into account below?

Mike Erwin (merwin) added inline comments.
source/blender/gpu/GPU_basic_shader.h
68

Add note to keep these in sync with gpu_shader_basic_frag.glsl

source/blender/gpu/shaders/gpu_shader_basic_frag.glsl
26

Add note to keep these in sync with GPU_basic_shader.h

Alexander Romanov (a.romanov) updated this revision to Diff 5631.
  • corrected in accordance with comments
Alexander Romanov (a.romanov) marked 7 inline comments as done.Dec 25 2015, 10:11 AM
  • modifications for BGE
  • fix wrong pattern name in GPU_basic_shader_stipple
  • no need for options here

The interlaced stereo drawing in the game engine seems like it's not going to work. The way that works currently is by enabling stipple and then having that affect all shaders used to draw the frame. However if we bind a GLSL shader with stipple drawing at the start of the frame, that's not going to have any effect when we bind other material shaders to replace it.

The window manager stereo drawing works because it first draws two views offscreen and the uses stipple to composite them. It seems like the game engine code stereo code should be rewritten to work similar, but that's out of scope for this task I think.

Besides that it basically looks good to me, I'll do some more testing and commit if all goes well.

source/blender/blenkernel/intern/editderivedmesh.c
806–807

This should use GPU_SHADER_USE_COLOR.

905–906

Same here.

source/blender/blenkernel/intern/subsurf_ccg.c
3654–3655

Same here.

source/blender/editors/screen/glutil.c
120–121

Same here.

source/blender/editors/uvedit/uvedit_draw.c
642–643

Same here.

718–719

Same here.

This revision was automatically updated to reflect the committed changes.

Committed now with one extra fix, a glDisable(GL_LINE_STIPPLE); had gone missing in metastrip drawing.