[Mesa-dev] [PATCH] i965: expose sRGB visuals and EGL_KHR_gl_colorspace

Tapani Pälli tapani.palli at intel.com
Mon Sep 4 16:57:52 UTC 2017



On 09/04/2017 07:13 PM, Jason Ekstrand wrote:
> A quick scan through and this looks pretty gross.  There may be no 
> better way to do it but I'm not sure with only a cursory glance.  Is 
> like this to wait on either me or Ken spending enough brain cells in it 
> to do a proper review.

Sure, no problem.

> 
> On September 4, 2017 6:12:20 AM Tapani Pälli <tapani.palli at intel.com> 
> wrote:
> 
>> Patch exposes sRGB visuals and adds DRI integer query support for
>> __DRI2_RENDERER_HAS_FRAMEBUFFER_SRGB. Further changes make sure that
>> we mark if the app explicitly wanted sRGB and for these framebuffers
>> we don't turn sRGB off in intel_gles3_srgb_workaround. This way we
>> keep compatibility for existing applications relying on default sRGB
>> and only add more visual support.
>>
>> With this change, following dEQP tests start to pass:
>>
>>    dEQP-EGL.functional.wide_color.window_8888_colorspace_srgb
>>    dEQP-EGL.functional.wide_color.pbuffer_8888_colorspace_srgb
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>
>> I did see following tests fail during CI run:
>>
>>  ES31-CTS.functional.blend_equation_advanced.srgb.colorburn.hswm64
>>  ES31-CTS.functional.blend_equation_advanced.basic.colordodge.hswm64
>>
>> However they don't seem to fail on non-hsw, so I'm not sure if this is
>> really a regression here? I saw that Ken has been burning some colors
>> before so I've CC:d him.
>>
>> Note, this might have some effect on following bug:
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=102503
>>
>> For me SuperTuxKart was working when testing (!) But it could be I was
>> testing a wrong version.
>>
>>  src/mesa/drivers/dri/i965/brw_context.c  | 16 ++++++++++------
>>  src/mesa/drivers/dri/i965/intel_fbo.h    |  5 +++++
>>  src/mesa/drivers/dri/i965/intel_screen.c | 12 ++++++++++++
>>  3 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
>> b/src/mesa/drivers/dri/i965/brw_context.c
>> index 6441311d47..a8e39e3bb6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> @@ -1112,8 +1112,8 @@ intelUnbindContext(__DRIcontext * driContextPriv)
>>   *
>>   * Unfortunately, renderbuffer setup happens before a context is 
>> created.  So
>>   * in intel_screen.c we always set up sRGB, and here, if you're a 
>> GLES2/3
>> - * context (without an sRGB visual, though we don't have sRGB visuals 
>> exposed
>> - * yet), we go turn that back off before anyone finds out.
>> + * context (without an sRGB visual), we go turn that back off before 
>> anyone
>> + * finds out.
>>   */
>>  static void
>>  intel_gles3_srgb_workaround(struct brw_context *brw,
>> @@ -1124,15 +1124,19 @@ intel_gles3_srgb_workaround(struct brw_context 
>> *brw,
>>     if (_mesa_is_desktop_gl(ctx) || !fb->Visual.sRGBCapable)
>>        return;
>>
>> -   /* Some day when we support the sRGB capable bit on visuals 
>> available for
>> -    * GLES, we'll need to respect that and not disable things here.
>> -    */
>> -   fb->Visual.sRGBCapable = false;
>>     for (int i = 0; i < BUFFER_COUNT; i++) {
>>        struct gl_renderbuffer *rb = fb->Attachment[i].Renderbuffer;
>> +
>> +      /* Check if sRGB was specifically asked for. */
>> +      struct intel_renderbuffer *irb = intel_get_renderbuffer(fb, i);
>> +      if (irb && irb->explicit_srgb)
>> +         return;
>> +
>>        if (rb)
>>           rb->Format = _mesa_get_srgb_format_linear(rb->Format);
>>     }
>> +   /* Disable sRGB from framebuffers that are not compatible. */
>> +   fb->Visual.sRGBCapable = false;
>>  }
>>
>>  GLboolean
>> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.h 
>> b/src/mesa/drivers/dri/i965/intel_fbo.h
>> index 1e2494286b..c8c2ed9a1b 100644
>> --- a/src/mesa/drivers/dri/i965/intel_fbo.h
>> +++ b/src/mesa/drivers/dri/i965/intel_fbo.h
>> @@ -116,6 +116,11 @@ struct intel_renderbuffer
>>      * for the duration of a mapping.
>>      */
>>     bool singlesample_mt_is_tmp;
>> +
>> +   /**
>> +    * Application specifically asked for a sRGB visual.
>> +    */
>> +   bool explicit_srgb;
>>  };
>>
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
>> b/src/mesa/drivers/dri/i965/intel_screen.c
>> index d39509bcb8..79cc962ab1 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -1341,6 +1341,9 @@ brw_query_renderer_integer(__DRIscreen *dri_screen,
>>     case __DRI2_RENDERER_HAS_TEXTURE_3D:
>>        value[0] = 1;
>>        return 0;
>> +   case __DRI2_RENDERER_HAS_FRAMEBUFFER_SRGB:
>> +      value[0] = 
>> screen->mesa_format_supports_render[MESA_FORMAT_B8G8R8A8_SRGB];
>> +      return 0;
>>     default:
>>        return driQueryRendererIntegerCommon(dri_screen, param, value);
>>     }
>> @@ -1486,12 +1489,16 @@ intelCreateBuffer(__DRIscreen *dri_screen,
>>        fb->Visual.samples = num_samples;
>>     }
>>
>> +   bool is_srgb = false;
>> +
>>     if (mesaVis->redBits == 5) {
>>        rgbFormat = mesaVis->redMask == 0x1f ? MESA_FORMAT_R5G6B5_UNORM
>>                                             : MESA_FORMAT_B5G6R5_UNORM;
>>     } else if (mesaVis->sRGBCapable) {
>>        rgbFormat = mesaVis->redMask == 0xff ? MESA_FORMAT_R8G8B8A8_SRGB
>>                                             : MESA_FORMAT_B8G8R8A8_SRGB;
>> +      /* mesaVis->sRGBCapable was set, user is asking for sRGB */
>> +      is_srgb = true;
>>     } else if (mesaVis->alphaBits == 0) {
>>        rgbFormat = mesaVis->redMask == 0xff ? MESA_FORMAT_R8G8B8X8_UNORM
>>                                             : MESA_FORMAT_B8G8R8X8_UNORM;
>> @@ -1504,10 +1511,12 @@ intelCreateBuffer(__DRIscreen *dri_screen,
>>     /* setup the hardware-based renderbuffers */
>>     rb = intel_create_winsys_renderbuffer(screen, rgbFormat, 
>> num_samples);
>>     _mesa_attach_and_own_rb(fb, BUFFER_FRONT_LEFT, &rb->Base.Base);
>> +   rb->explicit_srgb = is_srgb ? true : false;
>>
>>     if (mesaVis->doubleBufferMode) {
>>        rb = intel_create_winsys_renderbuffer(screen, rgbFormat, 
>> num_samples);
>>        _mesa_attach_and_own_rb(fb, BUFFER_BACK_LEFT, &rb->Base.Base);
>> +      rb->explicit_srgb = is_srgb ? true : false;
>>     }
>>
>>     /*
>> @@ -1854,6 +1863,9 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>>        MESA_FORMAT_B8G8R8A8_UNORM,
>>        MESA_FORMAT_B8G8R8X8_UNORM,
>>
>> +      MESA_FORMAT_B8G8R8A8_SRGB,
>> +      MESA_FORMAT_B8G8R8X8_SRGB,
>> +
>>        /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
>>         * Likewise for RGBX and BGRX.  Otherwise, the GLX client and 
>> the GLX
>>         * server may disagree on which format the GLXFBConfig represents,
>> -- 
>> 2.13.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 


More information about the mesa-dev mailing list