DRW: Colormanagment of Gizmos and Addon callback #74139

Closed
opened 2020-02-23 20:09:22 +01:00 by Clément Foucault · 16 comments

During overlay color management refactor (804e90b42d), we changed the rendering of overlay to a sRGB render target that ensure blending in linear space.

Overlays

This change some effects as blending was previously done in sRGB space which had a better perceptual progression.

  • Background gradient (fixed)
  • Wireframe facing gradient (fixed)
  • Edit Mesh facing gradient (fixed)
  • Bone Solid facing gradient (fixed)
  • Edit Mesh face overlays (need theme update) #73912
  • ... (likely more to come)

Theses needs a case by case fix. There is no workaround possible.

Addons

During the developpement of the patch we saw that refactoring the gpu shader to output linear color would be too much work.
We tried a workaround to disable sRGB GPU encoding when rendering to the overlay buffer for the gizmos and callbacks drawing.
However this breaks alpha blending in both gizmos and callbacks (addons), here is the reason:

Previously the callbacks and gizmos where rendered directly on top of the composited viewport image in an sRGB color space. Blending would occur in sRGB and blend with actual colored pixels. The blending would not be radiometrically correct but good enough in practice.

Now we render all the gizmos and callbacks to the same overlay buffer that only contains overlays with premultiplied (unassociated) alpha to allow a wide range of effects.

When doing compositing for the final blit to screen, we read the sRGB render target. This texture is automatically converted by opengl to linear color during sampling.
We do the compositing in linear color and transfer back the result to Display Encoded Color space (aka sRGB in most cases).

And this is where all the problem resides. See the gizmos were rendered with alpha blending in sRGB space. So the light ratios are correct if interpreting the resulting color in sRGB space.

But we are converting the color back to Linear space. The light ratios resulting of the masking don't match anymore and you get darker results in alpha blended region.

Example rendering white pixels over transparent background with Alpha 50% (this is the majority of the cases):

  • With blending in Linear (overlays current situation): RGB channels encode 50% of the Linear color. Encoding to sRGB is done after blending storing 73% intensity. Decoding correctly gives 50% intensity during the compositing phase.
  • With blending in sRGB (gizmo current situation): RGB channels encode 50% of the sRGB color. No encoding happen after blending. But loading the sRGB color in Linear space and you get 22% intensity.

IMPORTANT: The issue resides in the fact that we use both rendering mode on the same render target. And that we don't have the real final color "below" to interpolate in sRGB space like before to balance the "incorrectness" of it.

This issue was reported by @gfxcoder on twitter. https://twitter.com/gfxcoder/status/1231320641757962240

ERaEYRqWsAESNtz.jpg

So what choice do we have:

  • The right thing to do would be to do the hard work and port every gizmo shader (actually internal GPU shaders) to have a sRGB to linear transform before fragment shader output. But this inply more work and deeper refactors (i.e: how do we manage the shader variation). Ask addons devs to patch their custom shaders to output linear color. A simple pow(color, vec3(1.0 / 2.2)) could do it.
  • We live with the gizmos broken but still ask Addons to fix their shader. If an addon use an internal shader with blending it will stay broken.
  • A short term solution would be to use YET another buffer to output only gizmos and callbacks drawing and composite that during the blit to screen pass. I do not like this option as it adds overhead and increase memory usage.

IMPORTANT: Note that even if we fix the colorspace issue the blending will be different as it will happen in Linear space. Requiring manual tweaks to be done on the alpha value itself. Just like we did with the overlays.

The Fix

The quick hack:

out vec4 fragColor;

void main()
{
  /* Custom Shader code happening here. */
  [...]
  /* Transform to linear space */
  fragColor.rgb = pow(fragColor.rgb, vec3(1.0 / 2.2));
}

Another more correct approach would be to inject a function that do the right transform to all custom shaders and gizmo shaders.

out vec4 fragColor;

void main()
{
  /* Custom Shader code happening here. */
  [...]
  /* Transform to linear space */
  fragColor.rgb = srgb_to_linear(fragColor.rgb);
}

Note that for this fix to work we do need to remove this:

DefaultFramebufferList *dfbl = DRW_viewport_framebuffer_list_get();

 

DRW_state_reset();

 

GPU_framebuffer_bind(dfbl->overlay_fb);

- /* Disable sRGB encoding from the fixed function pipeline since all the drawing in this
- * function is done with sRGB color. Avoid double transform. */
- glDisable(GL_FRAMEBUFFER_SRGB);
 

GPU_matrix_projection_set(rv3d->winmat);
GPU_matrix_set(rv3d->viewmat);

 

/* annotations - temporary drawing buffer (3d space) */

During overlay color management refactor (804e90b42d), we changed the rendering of overlay to a sRGB render target that ensure blending in linear space. ## Overlays This change some effects as blending was previously done in sRGB space which had a better perceptual progression. - Background gradient (fixed) - Wireframe facing gradient (fixed) - Edit Mesh facing gradient (fixed) - Bone Solid facing gradient (fixed) - Edit Mesh face overlays (need theme update) #73912 - ... (likely more to come) Theses needs a case by case fix. There is no workaround possible. ## Addons During the developpement of the patch we saw that refactoring the gpu shader to output linear color would be too much work. We tried a workaround to disable sRGB GPU encoding when rendering to the overlay buffer for the gizmos and callbacks drawing. However this breaks alpha blending in both gizmos and callbacks (addons), here is the reason: Previously the callbacks and gizmos where rendered directly on top of the composited viewport image in an sRGB color space. Blending would occur in sRGB and blend with actual colored pixels. The blending would not be radiometrically correct but good enough in practice. Now we render all the gizmos and callbacks to the same overlay buffer that only contains overlays with premultiplied (unassociated) alpha to allow a wide range of effects. When doing compositing for the final blit to screen, we read the sRGB render target. This texture is automatically converted by opengl to linear color during sampling. We do the compositing in linear color and transfer back the result to Display Encoded Color space (aka sRGB in most cases). And this is where all the problem resides. See the gizmos were rendered with alpha blending in sRGB space. So the light ratios are correct if interpreting the resulting color in sRGB space. But we are converting the color back to Linear space. The light ratios resulting of the masking don't match anymore and you get darker results in alpha blended region. Example rendering white pixels over transparent background with Alpha 50% (this is the majority of the cases): - With blending in Linear (overlays current situation): RGB channels encode 50% of the Linear color. Encoding to sRGB is done after blending storing 73% intensity. Decoding correctly gives 50% intensity during the compositing phase. - With blending in sRGB (gizmo current situation): RGB channels encode 50% of the sRGB color. No encoding happen after blending. But loading the sRGB color in Linear space and you get 22% intensity. IMPORTANT: The issue resides in the fact that we use both rendering mode on the same render target. And that we don't have the real final color "below" to interpolate in sRGB space like before to balance the "incorrectness" of it. This issue was reported by @gfxcoder on twitter. https://twitter.com/gfxcoder/status/1231320641757962240 ![ERaEYRqWsAESNtz.jpg](https://archive.blender.org/developer/F8365051/ERaEYRqWsAESNtz.jpg) So what choice do we have: - The right thing to do would be to do the hard work and port every gizmo shader (actually internal GPU shaders) to have a sRGB to linear transform before fragment shader output. But this inply more work and deeper refactors (i.e: how do we manage the shader variation). Ask addons devs to patch their custom shaders to output linear color. A simple `pow(color, vec3(1.0 / 2.2))` could do it. - We live with the gizmos broken but still ask Addons to fix their shader. If an addon use an internal shader with blending it will stay broken. - A short term solution would be to use YET another buffer to output only gizmos and callbacks drawing and composite that during the blit to screen pass. I do not like this option as it adds overhead and increase memory usage. IMPORTANT: Note that even if we fix the colorspace issue the blending will be different as it will happen in Linear space. Requiring manual tweaks to be done on the alpha value itself. Just like we did with the overlays. ## The Fix The quick hack: ``` out vec4 fragColor; void main() { /* Custom Shader code happening here. */ [...] /* Transform to linear space */ fragColor.rgb = pow(fragColor.rgb, vec3(1.0 / 2.2)); } ``` Another more correct approach would be to inject a function that do the right transform to all custom shaders and gizmo shaders. ``` out vec4 fragColor; void main() { /* Custom Shader code happening here. */ [...] /* Transform to linear space */ fragColor.rgb = srgb_to_linear(fragColor.rgb); } ``` Note that for this fix to work we do need to remove this: ```@@ -1316,13 +1316,10 @@ void DRW_draw_callbacks_post_scene(void) ``` DefaultFramebufferList *dfbl = DRW_viewport_framebuffer_list_get(); ``` ``` DRW_state_reset(); ``` ``` GPU_framebuffer_bind(dfbl->overlay_fb); ``` - /* Disable sRGB encoding from the fixed function pipeline since all the drawing in this - * function is done with sRGB color. Avoid double transform. */ - glDisable(GL_FRAMEBUFFER_SRGB); ``` GPU_matrix_projection_set(rv3d->winmat); GPU_matrix_set(rv3d->viewmat); ``` ``` /* annotations - temporary drawing buffer (3d space) */ ```
Clément Foucault self-assigned this 2020-02-23 20:09:22 +01:00
Author
Member

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'
Author
Member

Added subscribers: @gfxcoder, @fclem

Added subscribers: @gfxcoder, @fclem
Member

Added subscriber: @EAW

Added subscriber: @EAW
Member

Added subscriber: @Imaginer

Added subscriber: @Imaginer

Added subscriber: @BYOB

Added subscriber: @BYOB

Added subscriber: @1D_Inc

Added subscriber: @1D_Inc

Ask addons devs to patch their custom shaders to output linear color. A simple pow(color, vec3(1.0 / 2.2)) could do it.

The sooner the better I guess. Low price for the right decision.

> Ask addons devs to patch their custom shaders to output linear color. A simple pow(color, vec3(1.0 / 2.2)) could do it. The sooner the better I guess. Low price for the right decision.
Member

Any update on what the plan is to solve this? I see that @fclem committed something for gizmos; does this fix gpu.shader.from_builtin and work for custom draw operators in addons as well?

Any update on what the plan is to solve this? I see that @fclem committed something for gizmos; does this fix `gpu.shader.from_builtin` and work for custom draw operators in addons as well?

Added subscriber: @brecht

Added subscriber: @brecht

D7261: GPUShader: Implement workaround for gizmo drawing on sRGB framebuffer is the fix for this but it has not been committed yet.

[D7261: GPUShader: Implement workaround for gizmo drawing on sRGB framebuffer](https://archive.blender.org/developer/D7261) is the fix for this but it has not been committed yet.
Author
Member

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'
Author
Member

Ok so I've commited the fix for addons.
The only change required is to use the new blender_srgb_to_framebuffer_space macro like this:
out_color = blender_srgb_to_framebuffer_space(out_color);
where out_color is the fragment shader output variable which is initially in srgb space.

If addons are using builtin shaders, they are already converted.

IMPORTANT: This will definitely still change the output color when using blending. In this case the color values will have to get manually tweaked to get the desired result.

Ok so I've commited the fix for addons. The only change required is to use the new `blender_srgb_to_framebuffer_space` macro like this: `out_color = blender_srgb_to_framebuffer_space(out_color);` where out_color is the fragment shader output variable which is initially in srgb space. If addons are using builtin shaders, they are already converted. IMPORTANT: This will definitely still change the output color when using blending. In this case the color values will have to get manually tweaked to get the desired result.

@fclem, be sure to add that information in the release notes:
https://wiki.blender.org/wiki/Reference/Release_Notes/2.83/Python_API

@fclem, be sure to add that information in the release notes: https://wiki.blender.org/wiki/Reference/Release_Notes/2.83/Python_API

Added subscriber: @lemenicier_julien

Added subscriber: @lemenicier_julien
Member

Is there a plan to add this to 2.90? Or does this not apply there?

Is there a plan to add this to 2.90? Or does this not apply there?
Member

Sorry, it's there. thanks for the work!

Sorry, it's there. thanks for the work!
Thomas Dinges added this to the 2.83 LTS milestone 2023-02-08 16:39:36 +01:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
8 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#74139
No description provided.