GPUShader: Implement workaround for gizmo drawing on sRGB framebuffer
ClosedPublic
Actions

Authored by Clément Foucault (fclem) on Mar 27 2020, 8:03 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Campbell Barton (campbellbarton)

Commits

rB21c658b718b9: GPUShader: Implement workaround for gizmo drawing on sRGB framebuffer

Summary

This solution involves adding a uniform to each fragment shader that is
used by gizmo drawing and use the framebuffer state to set this uniform
accordingly.

This solution can also be carried to external shaders (addons).
A single line of code would then be enough to fix the issue.

The only trickery here is the dummy define:
#define srgb_to_framebuffer_space(a)
This is in order to avoid breaking other DRW shaders that use the same
fragment shader code but do not need the tranformation.

Related to T74139

Diff Detail

Repository: rB Blender

Event Timeline

Clément Foucault (fclem) created this revision.Mar 27 2020, 8:03 PM

Harbormaster completed remote builds in B7347: Diff 23213.Mar 27 2020, 8:03 PM

Clément Foucault (fclem) edited reviewers, added: Campbell Barton (campbellbarton); removed: Jeroen Bakker (jbakker).Mar 27 2020, 8:12 PM

Would be nice to avoid adding function/uniform in the define, but I didn't see a convenient way to do this.

This revision is now accepted and ready to land.Mar 29 2020, 12:57 PM

How will this work for add-ons, can you add the Python API docs for that? It's not clear to me how that will work with a single line.

If srgb_to_framebuffer_space is to be used by add-ons, it should have some kind of prefix to make it clear this comes from Blender and to avoid naming conflicts with other uniforms that add-ons might use.

Move glsl code to separate file and add blender_ prefix
Implement solution for bpygpu users/addons

Harbormaster completed remote builds in B7362: Diff 23256.Mar 29 2020, 10:53 PM

I just tested with retopoflow addon and yes just adding blender_srgb_to_framebuffer_space(outColor); to the end of the fragment shader fixes the rendering.

Maybe we should prefer the fragOutput = blender_srgb_to_framebuffer_space(fragOutput); syntax? I have no opinion on this.

In D7261#174359, @Clément Foucault (fclem) wrote:

Maybe we should prefer the fragOutput = blender_srgb_to_framebuffer_space(fragOutput); syntax? I have no opinion on this.

I think that would be more clear.

source/blender/python/gpu/gpu_py_shader.c
612–613	This can be expanded I think. Something like: For drawing user interface elements and gizmos in the 3D viewport, use blender_srgb_to_framebuffer_space(fragOutput) to transform sRGB colors to the framebuffer colorspace. I'm not sure this is only for the 3D viewport, which is part of why this needs clarification.

This revision now requires changes to proceed.Mar 31 2020, 11:19 PM

I'm not sure this is only for the 3D viewport, which is part of why this needs clarification.

Well the 2D view is also a viewport too so I would encourage to always use it unless you are drawing to you own offscreen buffers.

Use return value instead of inout argument
Change python string var

Harbormaster completed remote builds in B7409: Diff 23350.Apr 1 2020, 11:37 PM

In D7261#175155, @Clément Foucault (fclem) wrote:

Well the 2D view is also a viewport too so I would encourage to always use it unless you are drawing to you own offscreen buffers.

This is still unclear to me. So is it only needed for the 3D viewport and image editor, and none of the other editors? And if you use it in other editors, will the result be wrong or will it just do nothing?

This revision is now accepted and ready to land.Apr 6 2020, 5:23 PM

In D7261#176182, @Brecht Van Lommel (brecht) wrote:

In D7261#175155, @Clément Foucault (fclem) wrote:

Well the 2D view is also a viewport too so I would encourage to always use it unless you are drawing to you own offscreen buffers.

This is still unclear to me. So is it only needed for the 3D viewport and image editor, and none of the other editors?