[Mesa-dev] [PATCH 6/6] st/mesa: use new cso_set_viewport_dims() helper

[Brian Paul]

On 02/12/2016 09:06 AM, Ilia Mirkin wrote:
> On Fri, Feb 12, 2016 at 10:43 AM, Brian Paul <brianp at vmware.com> wrote:
>> ---
>>   src/mesa/state_tracker/st_cb_bitmap.c     | 16 +++-------------
>>   src/mesa/state_tracker/st_cb_clear.c      | 13 ++-----------
>>   src/mesa/state_tracker/st_cb_drawpixels.c | 14 ++------------
>>   3 files changed, 7 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_bitmap.c b/src/mesa/state_tracker/st_cb_bitmap.c
>> index 5081639..5a11c0d 100644
>> --- a/src/mesa/state_tracker/st_cb_bitmap.c
>> +++ b/src/mesa/state_tracker/st_cb_bitmap.c
>> @@ -267,19 +267,9 @@ setup_render_state(struct gl_context *ctx,
>>      }
>>
>>      /* viewport state: viewport matching window dims */
>> -   {
>> -      const GLboolean invert = st->state.fb_orientation == Y_0_TOP;
>> -      const GLfloat width = (GLfloat)st->state.framebuffer.width;
>> -      const GLfloat height = (GLfloat)st->state.framebuffer.height;
>> -      struct pipe_viewport_state vp;
>> -      vp.scale[0] =  0.5f * width;
>> -      vp.scale[1] = height * (invert ? -0.5f : 0.5f);
>> -      vp.scale[2] = 0.5f;
>> -      vp.translate[0] = 0.5f * width;
>> -      vp.translate[1] = 0.5f * height;
>> -      vp.translate[2] = 0.5f;
>> -      cso_set_viewport(cso, &vp);
>> -   }
>> +   cso_set_viewport_dims(cso, st->state.framebuffer.width,
>> +                         st->state.framebuffer.height,
>> +                         st->state.fb_orientation == Y_0_TOP);
>>
>>      cso_set_vertex_elements(cso, 3, st->util_velems);
>>
>> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
>> index 271f577..27925f9 100644
>> --- a/src/mesa/state_tracker/st_cb_clear.c
>> +++ b/src/mesa/state_tracker/st_cb_clear.c
>> @@ -277,17 +277,8 @@ clear_with_quad(struct gl_context *ctx, unsigned clear_buffers)
>>      cso_set_rasterizer(cso, &st->clear.raster);
>>
>>      /* viewport state: viewport matching window dims */
>> -   {
>> -      const GLboolean invert = (st_fb_orientation(fb) == Y_0_TOP);
>> -      struct pipe_viewport_state vp;
>> -      vp.scale[0] = 0.5f * fb_width;
>> -      vp.scale[1] = fb_height * (invert ? -0.5f : 0.5f);
>> -      vp.scale[2] = 0.5f;
>> -      vp.translate[0] = 0.5f * fb_width;
>> -      vp.translate[1] = 0.5f * fb_height;
>> -      vp.translate[2] = 0.5f;
>> -      cso_set_viewport(cso, &vp);
>> -   }
>> +   cso_set_viewport_dims(st->cso_context, fb_width, fb_height,
>> +                         st_fb_orientation(fb) == Y_0_TOP);
>>
>>      set_fragment_shader(st);
>>      cso_set_tessctrl_shader_handle(cso, NULL);
>> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
>> index 581419e..b1b61c8 100644
>> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
>> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
>> @@ -596,18 +596,8 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z,
>>      }
>>
>>      /* viewport state: viewport matching window dims */
>> -   {
>> -      const float w = (float) ctx->DrawBuffer->Width;
>> -      const float h = (float) ctx->DrawBuffer->Height;
>> -      struct pipe_viewport_state vp;
>> -      vp.scale[0] =  0.5f * w;
>> -      vp.scale[1] = -0.5f * h;
>> -      vp.scale[2] = 0.5f;
>> -      vp.translate[0] = 0.5f * w;
>> -      vp.translate[1] = 0.5f * h;
>> -      vp.translate[2] = 0.5f;
>> -      cso_set_viewport(cso, &vp);
>> -   }
>> +   cso_set_viewport_dims(cso, ctx->DrawBuffer->Width,
>> +                         ctx->DrawBuffer->Height, TRUE);
>
> Please use _mesa_geometric_width/height here instead of
> ctx->DrawBuffer (to support the future ARB_framebuffer_no_attachments
> use-case). [I believe the current patchset for that feature touched
> this up, but since it's not in, might as well get it right...)

Can I do that in a follow-on patch?  I think the two changes should be 
separate.

BTW, if gl_framebuffer::_HasAttachments is false, that means the 
glDrawPixels will effectively be a no-op, right?  So the viewport size 
won't matter.

I see other places where we use gl_framebuffer::Width/Height in the 
state tracker code instead of _mesa_geometric_width/height().  I suspect 
it may be the same story.

Though, I guess it would be more consistent to use 
_mesa_geometric_width/height() everywhere.  What do you think?

-Brian