[Mesa-dev] [PATCH] i965: set VIEWPORT_BOUNDS_RANGE value depending of the supported OpenGL version

[Samuel Iglesias Gonsálvez]

On Tue, Mar 01, 2016 at 02:34:35AM -0800, Kenneth Graunke wrote:
> On Tuesday, March 1, 2016 9:48:16 AM PST Samuel Iglesias Gonsálvez wrote:
> > On Mon, Feb 29, 2016 at 04:33:42PM -0800, Kenneth Graunke wrote:
> > > On Friday, February 26, 2016 8:37:33 AM PST Samuel Iglesias Gonsálvez 
> wrote:
> > > > From ARB_viewport_array spec:
> > > > 
> > > > " * On GL3-capable hardware the VIEWPORT_BOUNDS_RANGE should be at least
> > > >     [-16384, 16383].
> > > >   * On GL4-capable hardware the VIEWPORT_BOUNDS_RANGE should be at least
> > > >     [-32768, 32767]."
> > > > 
> > > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_context.c | 13 +++++++++----
> > > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/
> dri/
> > > i965/brw_context.c
> > > > index 31b6b2a..1569992 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > > > @@ -687,10 +687,15 @@ brw_initialize_context_constants(struct 
> brw_context 
> > > *brw)
> > > >        ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
> > > >        ctx->Const.ViewportSubpixelBits = 0;
> > > >  
> > > > -      /* Cast to float before negating because MaxViewportWidth is 
> > > unsigned.
> > > > -       */
> > > > -      ctx->Const.ViewportBounds.Min = -(float)ctx-
> >Const.MaxViewportWidth;
> > > > -      ctx->Const.ViewportBounds.Max = ctx->Const.MaxViewportWidth;
> > > > +      if (brw->intelScreen->driScrnPriv->max_gl_core_version >= 40) {
> > > > +         ctx->Const.ViewportBounds.Min = -32768;
> > > > +         ctx->Const.ViewportBounds.Max = 32767;
> > > > +      } else {
> > > > +         /* Cast to float before negating because MaxViewportWidth is 
> > > unsigned.
> > > > +          */
> > > > +         ctx->Const.ViewportBounds.Min = -(float)ctx-
> > > >Const.MaxViewportWidth;
> > > > +         ctx->Const.ViewportBounds.Max = ctx->Const.MaxViewportWidth;
> > > > +      }
> > > >     }
> > > >  
> > > >     /* ARB_gpu_shader5 */
> > > > 
> > > 
> > > Shouldn't we just support a 32k x 32k viewport?  In other words, change
> > > MaxViewportWidth/MaxViewportHeight?
> > > 
> > 
> > Good question. I looked for the HW limitations for Viewport's width
> > and heigh but I couldn't find the reference in the PRMs, just that
> > it is a 32 bits float. I ran piglit with 32k x 32k viewport in ILK
> > and BDW and there is no regressions. But as I didn't know if piglit is
> > testing it correctly, I prefered to keep it unchanged. Nevertheless, I
> > am open to do this change.
> > 
> > If you agree, I will write a patch that sets MaxViewportWidth and 
> > MaxViewportHeight to 32k.
> > 
> > Sam
> 
> I tried to look them up too, briefly, and couldn't really find anything
> either.  I think the viewport is basically just an extra rectangle, and
> that the true limitations come from image (texture/renderbuffer) size
> restrictions.  I suspect we can bump it to 32k with no trouble.
> 
> I think we should do the following:
> 
> 1. Fix glInvalidateFramebuffer to use either ctx->Const.ViewportBounds 
>    or ctx->Const.MaxViewportWidth/Height instead of MAX_VIEWPORT_WIDTH
>    and MAX_VIEWPORT_HEIGHT (hardcoded #defines for 16384).  The spec
>    quote directly above this code makes it pretty obviously wrong.
> 2. Change Core Mesa to initialize ctx->Const.MaxViewportWidth to 16384.
> 3. Delete the MAX_VIEWPORT_WIDTH/HEIGHT #defines altogether.
> 4. Make i965 initialize ctx->Const.MaxViewportWidth/Height to 32768,
>    at least on Gen7+ which support GL 4.0.
> 
> (some of these changes could probably be squashed together if you like)
> 

Agreed. I am going to do them.

Thanks!

Sam

> Thanks for fixing these issues!
> 
> --Ken


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160301/bf54a6eb/attachment-0001.sig>