[Mesa-dev] [PATCH 08/16] mesa: Allow advanced blending enums in glBlendEquation[i].

[Francisco Jerez]

One comment not strictly related to this patch (or any other in
particular): Apparently the spec requires some amount of draw-time error
checking you don't seem to have implemented anywhere in this series,
e.g.:

|    If any non-NONE draw buffer uses a blend equation found in table X.1
|    or X.2, the error INVALID_OPERATION is generated by Begin or any
|    operation that implicitly calls Begin (such as DrawElements) if:
|
|      * the draw buffer for color output zero selects multiple color
|        buffers (e.g., FRONT_AND_BACK in the default framebuffer); or
|
|      * the draw buffer for any other color output is not NONE.

and:

|    If the current blend equation is found in table X.1 or X.2, and the
|    active fragment shader does not include the layout qualifier
|    matching the blend equation or "blend_support_all_equations", the
|    error INVALID_OPERATION is generated by [...]

Francisco Jerez <currojerez at riseup.net> writes:

> Kenneth Graunke <kenneth at whitecape.org> writes:
>
>> Don't allow them in glBlendEquationSeparate[i], though, as required
>> by the spec.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>
>> ---
>>  src/mesa/main/blend.c | 64 +++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 54 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
>> index 2ae22e9..fe83e59 100644
>> --- a/src/mesa/main/blend.c
>> +++ b/src/mesa/main/blend.c
>> @@ -336,11 +336,11 @@ _mesa_BlendFuncSeparateiARB(GLuint buf, GLenum sfactorRGB, GLenum dfactorRGB,
>>  
>>  
>>  /**
>> - * Check if given blend equation is legal.
>> - * \return GL_TRUE if legal, GL_FALSE otherwise.
>> + * Return true if \p mode is a legal blending equation, excluding
>> + * GL_KHR_blend_equation_advanced modes.
>>   */
>> -static GLboolean
>> -legal_blend_equation(const struct gl_context *ctx, GLenum mode)
>> +static bool
>> +legal_simple_blend_equation(const struct gl_context *ctx, GLenum mode)
>>  {
>>     switch (mode) {
>>     case GL_FUNC_ADD:
>> @@ -356,6 +356,36 @@ legal_blend_equation(const struct gl_context *ctx, GLenum mode)
>>  }
>>  
>>  
>> +/**
>> + * Return true if \p mode is one of the advanced blending equations
>> + * defined by GL_KHR_blend_equation_advanced.
>> + */
>> +static bool
>> +legal_advanced_blend_equation(const struct gl_context *ctx, GLenum mode)
>> +{
>> +   switch (mode) {
>> +   case GL_MULTIPLY_KHR:
>> +   case GL_SCREEN_KHR:
>> +   case GL_OVERLAY_KHR:
>> +   case GL_DARKEN_KHR:
>> +   case GL_LIGHTEN_KHR:
>> +   case GL_COLORDODGE_KHR:
>> +   case GL_COLORBURN_KHR:
>> +   case GL_HARDLIGHT_KHR:
>> +   case GL_SOFTLIGHT_KHR:
>> +   case GL_DIFFERENCE_KHR:
>> +   case GL_EXCLUSION_KHR:
>> +   case GL_HSL_HUE_KHR:
>> +   case GL_HSL_SATURATION_KHR:
>> +   case GL_HSL_COLOR_KHR:
>> +   case GL_HSL_LUMINOSITY_KHR:
>> +      return _mesa_has_KHR_blend_equation_advanced(ctx);
>> +   default:
>> +      return false;
>> +   }
>> +}
>> +
>> +
>>  /* This is really an extension function! */
>>  void GLAPIENTRY
>>  _mesa_BlendEquation( GLenum mode )
>> @@ -390,7 +420,8 @@ _mesa_BlendEquation( GLenum mode )
>>     if (!changed)
>>        return;
>>  
>> -   if (!legal_blend_equation(ctx, mode)) {
>> +   if (!legal_simple_blend_equation(ctx, mode) &&
>> +       !legal_advanced_blend_equation(ctx, mode)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquation");
>>        return;
>>     }
>> @@ -426,7 +457,8 @@ _mesa_BlendEquationiARB(GLuint buf, GLenum mode)
>>        return;
>>     }
>>  
>> -   if (!legal_blend_equation(ctx, mode)) {
>> +   if (!legal_simple_blend_equation(ctx, mode) &&
>> +       !legal_advanced_blend_equation(ctx, mode)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquationi");
>>        return;
>>     }
>> @@ -482,12 +514,18 @@ _mesa_BlendEquationSeparate( GLenum modeRGB, GLenum modeA )
>>        return;
>>     }
>>  
>> -   if (!legal_blend_equation(ctx, modeRGB)) {
>> +   /* Only allow simple blending equations.
>> +    * The GL_KHR_blend_equation_advanced spec says:
>> +    *
>> +    *    "NOTE: These enums are not accepted by the <modeRGB> or <modeAlpha>
>> +    *     parameters of BlendEquationSeparate or BlendEquationSeparatei."
>> +    */
>> +   if (!legal_simple_blend_equation(ctx, modeRGB)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquationSeparateEXT(modeRGB)");
>>        return;
>>     }
>>  
>> -   if (!legal_blend_equation(ctx, modeA)) {
>> +   if (!legal_simple_blend_equation(ctx, modeA)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquationSeparateEXT(modeA)");
>>        return;
>>     }
>> @@ -524,12 +562,18 @@ _mesa_BlendEquationSeparateiARB(GLuint buf, GLenum modeRGB, GLenum modeA)
>>        return;
>>     }
>>  
>> -   if (!legal_blend_equation(ctx, modeRGB)) {
>> +   /* Only allow simple blending equations.
>> +    * The GL_KHR_blend_equation_advanced spec says:
>> +    *
>> +    *    "NOTE: These enums are not accepted by the <modeRGB> or <modeAlpha>
>> +    *     parameters of BlendEquationSeparate or BlendEquationSeparatei."
>> +    */
>> +   if (!legal_simple_blend_equation(ctx, modeRGB)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquationSeparatei(modeRGB)");
>>        return;
>>     }
>>  
>> -   if (!legal_blend_equation(ctx, modeA)) {
>> +   if (!legal_simple_blend_equation(ctx, modeA)) {
>>        _mesa_error(ctx, GL_INVALID_ENUM, "glBlendEquationSeparatei(modeA)");
>>        return;
>>     }
>> -- 
>> 2.9.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160818/7ea06d58/attachment.sig>