mesa3d-dev

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Luca B. <luc...@gm...>]

Actually, why is the state tracker doing the min/max computation at all?

If the driver does the index lookup itself, as opposed to using an
hardware index buffer, (e.g. the nouveau drivers do this in some
cases) this is unnecessary and slow.

Would completely removing the call to vbo_get_minmax_index break anything?

Also, how about removing the max_index field in pipe_vertex_buffer?
This seems to be set to the same value for all vertex buffers, and the
value is then passed to draw_range_elements too.
Isn't the value passed to draw_range_elements sufficient?

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Luca B. <luc...@gm...>]

Isn't it possible to compute the maximum legal index by just taking
the minimum of:
(vb->buffer->size - vb->buffer_offset - ve->src_offset) / vb->stride

over all vertex buffers/elements?

Isn't the kernel checker doing something like this?

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Roland S. <sr...@vm...>]

On 13.03.2010 03:20, Corbin Simpson wrote:
> On Fri, Mar 12, 2010 at 5:20 PM, Jose Fonseca <jfo...@vm...>
> wrote:
>> Because if you have a huge vertex buffer and you only draw few
>> indices you may choose to upload to VRAM only the vertices actually
>> referred. Applications do this. And for certain hardware uploads
>> are very slow, so it is an worthwhile optimization.
> 
> Sure, not saying that it's not.
> 
>> Efficiency is just or more important goal than principles like
>> "state tracker should sanitize". There is hardware that can handle
>> buffers with out of bounds indices without crashing. The APIs we
>> expose also make the same promise. In such situation the
>> shortcomings on one hardware should not be taxed to all.
> 
> Oh, it's not a hardware limitation. If the vert and elt indices
> aren't clamped, then an attacker could theoretically retrieve bits of
>  VRAM/GTT he shouldn't be allowed to peek at. In order to prevent
> this, the radeon kernel module requires that I set those clamps, and
> that they be within the bounds of the various buffers being used for 
> rendering. I wouldn't call it a shortcoming so much as a security 
> consideration.
> 
> I've pushed a revert of the original patch, and an r300g patch that, 
> while not perfect, covers the common case that Wine hits.

I think I don't really understand that (and the fix you did for r300g).
Why can't you simply clamp the maxIndex to the minimum of the submitted
maxIndex and the vertex buffer max index?

Now you have this:
maxIndex = MIN3(maxIndex, r300->vertex_buffer_max_index, count - minIndex);

This is then used to set the hardware max index clamp. However, for
example count could be 3, min index 0, but the actual vertices fetched
0, 15, 30 - as long as the vertex buffers are large enough this is
perfectly legal, but as far as I can tell your patch would force the
hardware to just fetch the 0th vertex (3 times). Count really tells you
nothing at all about the index range (would also be legal to have huge
count but very small valid index range if you fetch same vertices
repeatedly).

Roland

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Corbin S. <mos...@gm...>]

On Fri, Mar 12, 2010 at 5:20 PM, Jose Fonseca <jfo...@vm...> wrote:
> Because if you have a huge vertex buffer and you only draw few indices you may choose to upload to VRAM only the vertices actually referred. Applications do this. And for certain hardware uploads are very slow, so it is an worthwhile optimization.

Sure, not saying that it's not.

> Efficiency is just or more important goal than principles like "state tracker should sanitize". There is hardware that can handle buffers with out of bounds indices without crashing. The APIs we expose also make the same promise. In such situation the shortcomings on one hardware should not be taxed to all.

Oh, it's not a hardware limitation. If the vert and elt indices aren't
clamped, then an attacker could theoretically retrieve bits of
VRAM/GTT he shouldn't be allowed to peek at. In order to prevent this,
the radeon kernel module requires that I set those clamps, and that
they be within the bounds of the various buffers being used for
rendering. I wouldn't call it a shortcoming so much as a security
consideration.

I've pushed a revert of the original patch, and an r300g patch that,
while not perfect, covers the common case that Wine hits.

~ C.

-- 
Only fools are easily impressed by what is only
barely beyond their reach. ~ Unknown

Corbin Simpson
<Mos...@gm...>

Re: [Mesa3d-dev] xdemos build breakage...

[Jeff S. <why...@ya...>]

>From: Dan Nicholson <dbn...@gm...>

>To: Brian Paul <br...@vm...>
>Cc: Jeff Smith <why...@ya...>; David Miller <da...@da...>; "mes...@li..." <mes...@li...>
>Sent: Fri, March 12, 2010 10:51:29 AM
>Subject: Re: [Mesa3d-dev] xdemos build breakage...
>
>>>That's not really the right thing, though. You're assuming that I have
>>>libX11 in the same libdir as I'm installing to and I want to use it.
>>>The fact is that configure uses pkg-config to check for x11 and other
>>>libraries needed to link the demos. It certainly was working before
>>>without requiring hardcoding things into the Makefiles.
>>
>> Oops, I didn't see your reply, Dan.  I already committed Jeff's patch.  If you have  better fix, please revert.
>
>No problem. I'll look at it a little later and see if there's more of
>a general fix from autoconf. I imagine it's not the last time we'll
>see build breakage in the demos.
>
>--
>Dan


Dan,
   Can you please review this patch?  I believe it handles the case described.

-- Jeff



      

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Jose F. <jfo...@vm...>]

What's the point of min/max ranges?

Because if you have a huge vertex buffer and you only draw few indices you may choose to upload to VRAM only the vertices actually referred. Applications do this. And for certain hardware uploads are very slow, so it is an worthwhile optimization.

Efficiency is just or more important goal than principles like "state tracker should sanitize". There is hardware that can handle buffers with out of bounds indices without crashing. The APIs we expose also make the same promise. In such situation the shortcomings on one hardware should not be taxed to all. 

Furthermore, if you need scan/clamp the indices in an index buffer then it might be worthwhile to do it in cached memory, and copy to uncached memory in the process. Often it is necessary to do software vertex processin fallbacks, so even for hardware that cannot do it there might draw calls where that's not necessary for the state tracker to do it. IMO, this entails too much hardware knowledge to be done both generally and efficiently by the state tracker.

The "state tracker should sanitize" principle derives from our desire to share code as much as possible. But there is more than one way to share code. In this case it should be a pip driver auxiliary library.

Jose

________________________________________
From: Corbin Simpson [mos...@gm...]
Sent: Saturday, March 13, 2010 0:06
To: Keith Whitwell
Cc: mes...@li...
Subject: Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid    index bounds.

This seems to be at odds with the idea that the pipe driver receives pre-sanitized inputs. If this patch is reverted:
- I can't trust the min and max elts, because they're set to ~0
- I can't just use the vertex buffer sizes, because they're set to ~0

So I have to do the maths myself, guessing just like st/mesa guesses. Maybe this is specific to Radeons, but I *always* need those values.

I don't mind this, but IMO it *must* be doc'd, "The minimum and maximum index limits passed to draw_elements and draw_range_elements may be invalid," and really, at that point, why are we even passing them? Seems absurd to me. :3

Posting from a mobile, pardon my terseness. ~ C.

On Mar 12, 2010 5:40 AM, "Keith Whitwell" <ke...@vm...<mailto:ke...@vm...>> wrote:

On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
> Module: Mesa
> Branch: master
> Commit: 50876ddaaff72a324ac45e255985e0f84e108594
> URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
>
> Author: Corbin Simpson <Mos...@gm...<mailto:Mos...@gm...>>
> Date:   Fri Mar 12 02:51:40 2010 -0800
>
> st/mesa: Always recalculate invalid index bounds.
>
> These should always be sanitized before heading towards the pipe driver,
> and if the calling function explicitly marked them as invalid, we need
> to regenerate them.
>
> Allows r300g to properly pass a bit more of Wine's d3d9 testing without
> dropping stuff on the floor.
>
> ---
>
>  src/mesa/state_tracker/st_draw.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
> index 8b01272..d81b361 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
>     assert(ctx->NewState == 0x0);
>
>     /* Gallium probably doesn't want this in some cases. */
> -   if (!index_bounds_valid)
> -      if (!vbo_all_varyings_in_vbos(arrays))
> -      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +   if (index_bounds_valid != GL_TRUE) {
> +      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +   }


Corbin,

This isn't really desirable.  Ideally this range checking should be
pushed into pipe driver, because it's an expensive operation that is not
necessary on a lot of hardware.  #

Specifically, vertex fetch hardware can often be set up with the min/max
permissible index bounds to avoid accessing vertex data outside of the
bound VB's.  This can be achieved by examining the VBs, with min_index
== 0, max_index = vb.size / vb.stride.

The case where we need to calculate them internally is if some data is
not in a VB, meaning we can't guess what the legal min/max values are.
Also note that we need that min/max information to be able to upload the
data to a VB.

So, I think the code was probably correct in the original version -
defer the minmax scan to the hardware driver, which may or may not need
it.

But maybe there is a better way to let the driver know that we are not
providing this information.

Keith

Re: [Mesa3d-dev] Cast needed in program_lexer.l ???

[Karl S. <kar...@gm...>]

On Fri, Mar 12, 2010 at 5:44 PM, Gonsolo <go...@gm...> wrote:

> We can't use strtof for the same reason we can't (directly) use strtod.
>>  It uses the locale, so that can make it parse numbers incorrectly.  All
>> of the users of _mesa_strtod actually want _mesa_strtof, so the right
>> fix is to change the existing _mesa_strtod to _mesa_strtof.
>>
>
> Some remarks from a beginner:
>
> 1. Isn't strtod meant to be a very basic function? Making it dependent on a
> locale isn't helpful.
>
> 2. I see strtod_l is defined in /usr/include/stdlib.h which is part of the
> package libc6-dev on my Ubuntu system, yet there is no manpage for it.
>
> 3. strtod is defined in /usr/include/stdlib.h which is in package
> libc6-dev, the manpage is in manpages-dev.
>
> From my limited view, every function should be defined in some_package, it
> should be declared in some_package-dev and documented in some_package-doc.
>
> Is there any reason for this mess except being "historical"?
>
> (Please excuse my ignorance, just a beginner :) )
>
> g
>

The code in question is:

/** Wrapper around strtod() */
double
_mesa_strtod( const char *s, char **end )
{
#ifdef _GNU_SOURCE
   static locale_t loc = NULL;
   if (!loc) {
      loc = newlocale(LC_CTYPE_MASK, "C", NULL);
   }
   return strtod_l(s, end, loc);
#else
   return strtod(s, end);
#endif
}

In some locales, a comma can be used as a decimal place.  That is, 5,2 is a
number between 5 and 6.  (I think I have that right)  I would guess that the
shader language, like C, wouldn't allow this form in code.  So, it makes
sense to force the C locale when parsing numbers from shader source code, as
the code does above.

strtof doesn't show up until C99 and not all compilers support it, including
the MSFT Windows compilers.  Ian says that all usages of this function want
a float anyway, so we may end up with something like:

float
_mesa_strtof( const char *s, char **end )
{
#ifdef _GNU_SOURCE
   static locale_t loc = NULL;
   if (!loc) {
      loc = newlocale(LC_CTYPE_MASK, "C", NULL);
   }
   return (float) strtod_l(s, end, loc);
#else
   return (float) strtod(s, end);
#endif
}

And then change all _mesa_strtod to _mesa_strtof.

If Ian doesn't care for the casts here, then I'm fine with silencing
warnings in the Studio with a compiler option.

Re: [Mesa3d-dev] Cast needed in program_lexer.l ???

[Gonsolo <go...@gm...>]

> We can't use strtof for the same reason we can't (directly) use strtod.
>   It uses the locale, so that can make it parse numbers incorrectly.  All
> of the users of _mesa_strtod actually want _mesa_strtof, so the right
> fix is to change the existing _mesa_strtod to _mesa_strtof.

Some remarks from a beginner:

1. Isn't strtod meant to be a very basic function? Making it dependent 
on a locale isn't helpful.

2. I see strtod_l is defined in /usr/include/stdlib.h which is part of 
the package libc6-dev on my Ubuntu system, yet there is no manpage for it.

3. strtod is defined in /usr/include/stdlib.h which is in package 
libc6-dev, the manpage is in manpages-dev.

 From my limited view, every function should be defined in some_package, 
it should be declared in some_package-dev and documented in 
some_package-doc.

Is there any reason for this mess except being "historical"?

(Please excuse my ignorance, just a beginner :) )

g

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Brian P. <bri...@gm...>]

The overriding goal is to avoid the expense of computing this in
software when the hardware can do the bounds check.

I guess we could add a PIPE_CAP query to ask the driver if it can do
bounds checking and if it can't, do it in the state tracker.  But I
think Keith's interested in minimizing queries like this.  Hence, try
to do it in the driver.  I think it should be easy to write a
re-usable utility function to do the checks.

If you're seeing ~0, that means the Mesa VBO module has not computed
the bounds, and/or the user didn't provide them. I'd have to check the
code.

If vertex data is coming from regular memory and not a VBO we have no
idea what the legal bounds are.

-Brian


On Fri, Mar 12, 2010 at 5:06 PM, Corbin Simpson
<mos...@gm...> wrote:
> This seems to be at odds with the idea that the pipe driver receives
> pre-sanitized inputs.
>
> If this patch is reverted:
> - I can't trust the min and max elts, because they're set to ~0
> - I can't just use the vertex buffer sizes, because they're set to ~0
>
> So I have to do the maths myself, guessing just like st/mesa guesses. Maybe
> this is specific to Radeons, but I *always* need those values.
>
> I don't mind this, but IMO it *must* be doc'd, "The minimum and maximum
> index limits passed to draw_elements and draw_range_elements may be
> invalid," and really, at that point, why are we even passing them? Seems
> absurd to me. :3
>
> Posting from a mobile, pardon my terseness. ~ C.
>
> On Mar 12, 2010 5:40 AM, "Keith Whitwell" <ke...@vm...> wrote:
>
> On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
>> Module: Mesa
>> Branch: master
>> Commit: 50876ddaaff72a324ac45e255985e0f84e108594
>> URL:
>>  http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
>>
>> Author: Corbin Simpson <Mos...@gm...>
>> Date:   Fri Mar 12 02:51:40 2010 -0800
>>
>> st/mesa: Always recalculate invalid index bounds.
>>
>> These should always be sanitized before heading towards the pipe driver,
>> and if the calling function explicitly marked them as invalid, we need
>> to regenerate them.
>>
>> Allows r300g to properly pass a bit more of Wine's d3d9 testing without
>> dropping stuff on the floor.
>>
>> ---
>>
>>  src/mesa/state_tracker/st_draw.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_draw.c
>> b/src/mesa/state_tracker/st_draw.c
>> index 8b01272..d81b361 100644
>> --- a/src/mesa/state_tracker/st_draw.c
>> +++ b/src/mesa/state_tracker/st_draw.c
>> @@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
>>     assert(ctx->NewState == 0x0);
>>
>>     /* Gallium probably doesn't want this in some cases. */
>> -   if (!index_bounds_valid)
>> -      if (!vbo_all_varyings_in_vbos(arrays))
>> -      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
>> +   if (index_bounds_valid != GL_TRUE) {
>> +      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
>> +   }
>
>
> Corbin,
>
> This isn't really desirable.  Ideally this range checking should be
> pushed into pipe driver, because it's an expensive operation that is not
> necessary on a lot of hardware.  #
>
> Specifically, vertex fetch hardware can often be set up with the min/max
> permissible index bounds to avoid accessing vertex data outside of the
> bound VB's.  This can be achieved by examining the VBs, with min_index
> == 0, max_index = vb.size / vb.stride.
>
> The case where we need to calculate them internally is if some data is
> not in a VB, meaning we can't guess what the legal min/max values are.
> Also note that we need that min/max information to be able to upload the
> data to a VB.
>
> So, I think the code was probably correct in the original version -
> defer the minmax scan to the hardware driver, which may or may not need
> it.
>
> But maybe there is a better way to let the driver know that we are not
> providing this information.
>
> Keith
>
>
>
>
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Mesa3d-dev mailing list
> Mes...@li...
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
>
>

Re: [Mesa3d-dev] Framebuffers in mesa

[Brian P. <bri...@gm...>]

On Fri, Mar 12, 2010 at 12:19 PM, Maciej Cencora <m.c...@gm...> wrote:
> Hi all,
>
> I've got some questions regarding FBOs in mesa. I hope you'll be able to
> answer at least some of them.
>
> GLcontext structure holds pointers to 4 gl_framebuffers (DrawBuffer, ReadBuffer,
> WinSysDrawBuffer, WinSysReadBuffer) and one to gl_renderbuffer
> (CurrentRenderbuffer). gl_framebuffer struct contains amongs other fields:
> _ColorDrawBuffers[], _ColorReadBuffers, _DepthBuffer, _StencilBuffer.
> Now having in mind that r300 wraps all gl_renderbuffers and gl_texture_object


Do you mean that r300 subclasses gl_renderbuffer and
gl_texture_object?  That's what I think you mean.  The term "wrapping"
has a special meaning for renderbuffers (see below).


> (they contain HW buffer objects that I'm interested in), what is the proper way
> to get to read and draw buffers?

The current "read" framebuffer is at ctx->ReadBuffer.  The current
drawing framebuffer is ctx->DrawBuffer.

Then, if you're reading _colors_ the renderbuffer to use is
ctx->ReadBuffer->_ColorReadBuffer.  If you want to read _stencil_
values, it would be ctx->ReadBuffer->_StencilBuffer.  Similarly for
Z/depth, etc.

To draw to color buffers (there may be more than one) you'll want
ctx->DrawBuffer->_ColorDrawBuffers[].  For stencil it's
ctx->DrawBuffer->_Stencil, etc.

Renderbuffers typically correspond to a region of video memory.  How
that works is driver-dependent.


> What operations use ctx->_ReadBuffer besides glReadPixels,glCopyPixels and
> glCopyTex(Sub)Image?

glCopyColorTable and maybe some other obscure functions.  You got the main ones.


> What operations use ctx->_DrawBuffer besides rendering operations?
> (glBegin/glEnd, glDrawElements, glDrawArrays, ...)

That's basically it, plus glClear.


> Am I correct that for ctx->_DrawBuffer field _ColorReadBuffer is unused, and for
> ctx->_ReadBuffer _ColorDrawBuffers is unused?

No, they're used - see above.  The _ColorReadBuffer field depends on
the glReadBuffer() call.  The _ColorDrawBuffers[] pointers depend on
the glDrawBuffer() and glDrawBuffersARB() functions.


It's important to understand the heirarchy of gl_framebuffers and
gl_renderbuffers.

Each framebuffer object has a collection of renderbuffers.

Renderbuffers may also act as wrappers for gl_texture_object when
doing render to texture.  In this case, the renderbuffer acts as a 2D
view into a level/slice/face of a texture object.

Also, we have special depth/stencil renderbuffers which wrap combined
depth/stencil buffers.  See main/depthstencil.c  That's mainly for
software rendering, though.

-Brian

[Mesa3d-dev] [PATCH] nv50: fixup after gallium-context-transfers merge

[Xavier C. <cha...@gm...>]

Signed-off-by: Xavier Chantry <cha...@gm...>
---
 src/gallium/drivers/nv50/nv50_screen.c |    1 -
 src/gallium/drivers/nv50/nv50_screen.h |    2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nv50/nv50_screen.c b/src/gallium/drivers/nv50/nv50_screen.c
index 7e2e8aa..adf0d3b 100644
--- a/src/gallium/drivers/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nv50/nv50_screen.c
@@ -234,7 +234,6 @@ nv50_screen_create(struct pipe_winsys *ws, struct nouveau_device *dev)
 	pscreen->context_create = nv50_create;
 
 	nv50_screen_init_miptree_functions(pscreen);
-	nv50_transfer_init_screen_functions(pscreen);
 
 	/* DMA engine object */
 	ret = nouveau_grobj_alloc(chan, 0xbeef5039,
diff --git a/src/gallium/drivers/nv50/nv50_screen.h b/src/gallium/drivers/nv50/nv50_screen.h
index d1bc80c..ec19ea6 100644
--- a/src/gallium/drivers/nv50/nv50_screen.h
+++ b/src/gallium/drivers/nv50/nv50_screen.h
@@ -38,6 +38,4 @@ nv50_screen(struct pipe_screen *screen)
 	return (struct nv50_screen *)screen;
 }
 
-void nv50_transfer_init_screen_functions(struct pipe_screen *);
-
 #endif
-- 
1.7.0.1

Re: [Mesa3d-dev] Mesa (master): st/mesa: Always recalculate invalid index bounds.

[Corbin S. <mos...@gm...>]

This seems to be at odds with the idea that the pipe driver receives
pre-sanitized inputs. If this patch is reverted:
- I can't trust the min and max elts, because they're set to ~0
- I can't just use the vertex buffer sizes, because they're set to ~0

So I have to do the maths myself, guessing just like st/mesa guesses. Maybe
this is specific to Radeons, but I *always* need those values.

I don't mind this, but IMO it *must* be doc'd, "The minimum and maximum
index limits passed to draw_elements and draw_range_elements may be
invalid," and really, at that point, why are we even passing them? Seems
absurd to me. :3

Posting from a mobile, pardon my terseness. ~ C.

On Mar 12, 2010 5:40 AM, "Keith Whitwell" <ke...@vm...> wrote:

On Fri, 2010-03-12 at 02:54 -0800, Corbin Simpson wrote:
> Module: Mesa
> Branch: master
> Commit: 50876ddaaff72a324ac45e255985e0f84e108594
> URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=50876ddaaff72a324ac45e255985e0f84e108594
>
> Author: Corbin Simpson <Mos...@gm...>
> Date:   Fri Mar 12 02:51:40 2010 -0800
>
> st/mesa: Always recalculate invalid index bounds.
>
> These should always be sanitized before heading towards the pipe driver,
> and if the calling function explicitly marked them as invalid, we need
> to regenerate them.
>
> Allows r300g to properly pass a bit more of Wine's d3d9 testing without
> dropping stuff on the floor.
>
> ---
>
>  src/mesa/state_tracker/st_draw.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_draw.c
b/src/mesa/state_tracker/st_draw.c
> index 8b01272..d81b361 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -542,9 +542,9 @@ st_draw_vbo(GLcontext *ctx,
>     assert(ctx->NewState == 0x0);
>
>     /* Gallium probably doesn't want this in some cases. */
> -   if (!index_bounds_valid)
> -      if (!vbo_all_varyings_in_vbos(arrays))
> -      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +   if (index_bounds_valid != GL_TRUE) {
> +      vbo_get_minmax_index(ctx, prims, ib, &min_index, &max_index);
> +   }


Corbin,

This isn't really desirable.  Ideally this range checking should be
pushed into pipe driver, because it's an expensive operation that is not
necessary on a lot of hardware.  #

Specifically, vertex fetch hardware can often be set up with the min/max
permissible index bounds to avoid accessing vertex data outside of the
bound VB's.  This can be achieved by examining the VBs, with min_index
== 0, max_index = vb.size / vb.stride.

The case where we need to calculate them internally is if some data is
not in a VB, meaning we can't guess what the legal min/max values are.
Also note that we need that min/max information to be able to upload the
data to a VB.

So, I think the code was probably correct in the original version -
defer the minmax scan to the hardware driver, which may or may not need
it.

But maybe there is a better way to let the driver know that we are not
providing this information.

Keith

Re: [Mesa3d-dev] Cast needed in program_lexer.l ???

[Ian R. <id...@fr...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gonsolo wrote:

> diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
> index 56e8195..d77b36c 100644
> --- a/src/mesa/main/imports.c
> +++ b/src/mesa/main/imports.c
> @@ -810,6 +810,13 @@ _mesa_strtod( const char *s, char **end )
>  #endif
>  }
> 
> +/** Wrapper around strtof() */
> +float
> +_mesa_strtof( const char *s, char **end )
> +{
> +   return strtof(s, end);
> +}
> +

We can't use strtof for the same reason we can't (directly) use strtod.
 It uses the locale, so that can make it parse numbers incorrectly.  All
of the users of _mesa_strtod actually want _mesa_strtof, so the right
fix is to change the existing _mesa_strtod to _mesa_strtof.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkua0dEACgkQX1gOwKyEAw9kNgCffpzl4Zxk2JjOwAjIhR2gpiEp
Y8IAoIs8IjQdhm7RdOmmYqWk/RylSmYK
=0Ep5
-----END PGP SIGNATURE-----

Re: [Mesa3d-dev] Cast needed in program_lexer.l ???

[Gonsolo <go...@gm...>]

> Absolutely not.  All the casts do is clutter the code.  This is a
> completely worthless warning from Visual Studio.  It generates only
> noise.  I have never seen a case where it was actually useful.  There
> are pragmas and command line options to disable specific warnings.  You
> should do that.
>
> Brian, could you please revert those patches.  I don't want that crap in
> code that I maintain.  Seriously.

What about this patch:

 From 40136f460562d49a21877fdb52d9ddbf8ae6f7b9 Mon Sep 17 00:00:00 2001
From: Gon Solo <go...@gm...>
Date: Fri, 12 Mar 2010 21:39:59 +0100
Subject: [PATCH] Use strof. C99 is 11 years now.

---
  src/mesa/main/imports.c         |    7 +++++++
  src/mesa/main/imports.h         |    3 +++
  src/mesa/shader/lex.yy.c        |    8 ++++----
  src/mesa/shader/program_lexer.l |    8 ++++----
  4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 56e8195..d77b36c 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -810,6 +810,13 @@ _mesa_strtod( const char *s, char **end )
  #endif
  }

+/** Wrapper around strtof() */
+float
+_mesa_strtof( const char *s, char **end )
+{
+   return strtof(s, end);
+}
+
  /** Compute simple checksum/hash for a string */
  unsigned int
  _mesa_str_checksum(const char *str)
diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h
index fb4a00e..c30be11 100644
--- a/src/mesa/main/imports.h
+++ b/src/mesa/main/imports.h
@@ -578,6 +578,9 @@ _mesa_strdup( const char *s );
  extern double
  _mesa_strtod( const char *s, char **end );

+extern float
+_mesa_strtof( const char *s, char **end );
+
  extern unsigned int
  _mesa_str_checksum(const char *str);

diff --git a/src/mesa/shader/lex.yy.c b/src/mesa/shader/lex.yy.c
index d1af35f..6d09d1d 100644
--- a/src/mesa/shader/lex.yy.c
+++ b/src/mesa/shader/lex.yy.c
@@ -2212,7 +2212,7 @@ case 142:
  YY_RULE_SETUP
  #line 326 "program_lexer.l"
  {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  	YY_BREAK
@@ -2224,7 +2224,7 @@ YY_DO_BEFORE_ACTION; /* set up yytext again */
  YY_RULE_SETUP
  #line 330 "program_lexer.l"
  {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  	YY_BREAK
@@ -2232,7 +2232,7 @@ case 144:
  YY_RULE_SETUP
  #line 334 "program_lexer.l"
  {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  	YY_BREAK
@@ -2240,7 +2240,7 @@ case 145:
  YY_RULE_SETUP
  #line 338 "program_lexer.l"
  {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  	YY_BREAK
diff --git a/src/mesa/shader/program_lexer.l 
b/src/mesa/shader/program_lexer.l
index 83bc508..fe18272 100644
--- a/src/mesa/shader/program_lexer.l
+++ b/src/mesa/shader/program_lexer.l
@@ -324,19 +324,19 @@ ARRAYSHADOW2D             { 
return_token_or_IDENTIFIER(require_ARB_fp && require
     return INTEGER;
  }
  {num}?{frac}{exp}?        {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  {num}"."/[^.]             {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  {num}{exp}                {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }
  {num}"."{exp}             {
-   yylval->real = _mesa_strtod(yytext, NULL);
+   yylval->real = _mesa_strtof(yytext, NULL);
     return REAL;
  }

-- 
1.6.3.3

[Mesa3d-dev] [Bug 23941] Wine offscreen rendering causes GL_INVALID_FRAMEBUFFER_OPERATION_EXT

[<bug...@fr...>]

http://bugs.freedesktop.org/show_bug.cgi?id=23941


Sven Arvidsson <sa...@wh...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #4 from Sven Arvidsson <sa...@wh...>  2010-03-12 12:29:11 PST ---
This is working a lot better now, both games in Wine in general, and the
specific title I used for testing this (the Halo demo). 

Both the rendering errors and the warning message is gone. 


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Re: [Mesa3d-dev] Cast needed in program_lexer.l ???

[Ian R. <id...@fr...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Karl Schultz wrote:
> Am getting these warnings in the Windows build:
> 
> 2>Compiling...
> 2>lex.yy.c
> 2>program_lexer.l(327) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(331) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(335) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data
> 2>program_lexer.l(339) : warning C4244: '=' : conversion from 'double'
> to 'float', possible loss of data

[snip]

> If it is not a huge hassle, can someone add these casts to 7.8 and
> master?  The warnings are no big deal, but the casts do have some value
> in documenting the implicit conversion.

Absolutely not.  All the casts do is clutter the code.  This is a
completely worthless warning from Visual Studio.  It generates only
noise.  I have never seen a case where it was actually useful.  There
are pragmas and command line options to disable specific warnings.  You
should do that.

Brian, could you please revert those patches.  I don't want that crap in
code that I maintain.  Seriously.

Thanks.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuanyEACgkQX1gOwKyEAw/PJwCgjZDqwdLrkC5hZ28hCgLpdmvH
anYAnAm2rMb2fSgnOjsqRSybuhQ1ZmOA
=cRwl
-----END PGP SIGNATURE-----

[Mesa3d-dev] Framebuffers in mesa

[Maciej C. <m.c...@gm...>]

Hi all,

I've got some questions regarding FBOs in mesa. I hope you'll be able to 
answer at least some of them.

GLcontext structure holds pointers to 4 gl_framebuffers (DrawBuffer, ReadBuffer, 
WinSysDrawBuffer, WinSysReadBuffer) and one to gl_renderbuffer 
(CurrentRenderbuffer). gl_framebuffer struct contains amongs other fields:
_ColorDrawBuffers[], _ColorReadBuffers, _DepthBuffer, _StencilBuffer.
Now having in mind that r300 wraps all gl_renderbuffers and gl_texture_object 
(they contain HW buffer objects that I'm interested in), what is the proper way 
to get to read and draw buffers?

What operations use ctx->_ReadBuffer besides glReadPixels,glCopyPixels and 
glCopyTex(Sub)Image?

What operations use ctx->_DrawBuffer besides rendering operations? 
(glBegin/glEnd, glDrawElements, glDrawArrays, ...)

Am I correct that for ctx->_DrawBuffer field _ColorReadBuffer is unused, and for 
ctx->_ReadBuffer _ColorDrawBuffers is unused?

Regards,
Maciej

Re: [Mesa3d-dev] xdemos build breakage...

[Dan N. <dbn...@gm...>]

On Fri, Mar 12, 2010 at 8:13 AM, Brian Paul <br...@vm...> wrote:
> Dan Nicholson wrote:
>>On Fri, Mar 12, 2010 at 6:48 AM, Jeff Smith <why...@ya...> wrote:
>>>> This change:
>>>>
>>>> commit d888bbc45a84946cafb4f4d2c89681a580cd89bc
>>>> Author: Brian Paul <br...@vm...>
>>>> Date:   Tue Nov 17 13:39:13 2009 -0700
>>>>
>>>>    progs/xdemos: added -lX11 -lpthread for GNU gold linker
>>>>
>>>> breaks the build if you are building under a specific path prefix
>>>> (say, /opt/XORG) and you have an existing X11 installation in the
>>>> usual location (/usr, etc.)
>>>
>>> I found the same problem and sent a patch to this list a few hours ago to address it:
>>>
>>> "[PATCH] Add -L$(libdir) for xdemos and egl so that the right libX11 is found"
>>
>>That's not really the right thing, though. You're assuming that I have
>>libX11 in the same libdir as I'm installing to and I want to use it.
>>The fact is that configure uses pkg-config to check for x11 and other
>>libraries needed to link the demos. It certainly was working before
>>without requiring hardcoding things into the Makefiles.
>
> Oops, I didn't see your reply, Dan.  I already committed Jeff's patch.  If you have  better fix, please revert.

No problem. I'll look at it a little later and see if there's more of
a general fix from autoconf. I imagine it's not the last time we'll
see build breakage in the demos.

--
Dan

Re: [Mesa3d-dev] util_format cleanup leftover: Gallium BGRA vertex swizzles

[José F. <jfo...@vm...>]

On Fri, 2010-03-12 at 08:17 -0800, Brian Paul wrote:
> Jose wrote:
> > On Thu, 2010-03-11 at 17:50 -0800, Marek Olšák wrote:
> > > Please see this commit:
> > > http://cgit.freedesktop.org/~mareko/mesa/commit/?id=177a4432aa88fc68d83895ae71b7ad2c86a1e7e3
> > >
> > > Subject: [PATCH] gallium: fix BGRA vertex color swizzles
> > >
> > > The mapping for vertex_array_bgra:
> > > (gl -> st -> translate)
> > > GL_RGBA -> PIPE_FORMAT_R8G8B8A8 (RGBA) -> no swizzle (XYZW)
> > > GL_BGRA -> PIPE_FORMAT_A8R8G8B8 (ARGB) -> ZYXW (BGRA again??)
> > >
> > > Iẗ́'s pretty clear that PIPE_FORMAT_A8R8G8B8 here is wrong. This
> > > commit
> > > fixes the pipe format and removes obvious workarounds in
> > > util/translate.
> > >
> > > Tested with: softpipe, llvmpipe, r300g.
> > >
> > > Please review.
> > 
> > Marek,
> > 
> > Well spotted.
> > 
> > These were cases where formats were being used inconsistently in respect
> > to their LSB->MSB vs MSB->LSB meanings and my automatic format rename
> > did no more than flipping the inconsistency the otherway around.
> > 
> > Looks good to me.
> > 
> > I've went ahead and commited since I needed to fix up other state
> > trackers.
> 
> Should this go into the 7.8 branch too, Jose?

It's a cleanup -- it doesn't actually change behavior --, but I think it
is better  to port to 7.8 since cherrypicking future related fixes could
end up having different effect on 7.8 branch.

Jose

[Mesa3d-dev] New tinderbox regression (gallium)

[Chris B. <cj...@la...>]

Hi,

http://tinderbox.x.org/builds/2010-03-12-0016/logs/libGL/#build

In file included from xlib_sw_winsys.c:42:
../../../../src/gallium/include/state_tracker/xlib_sw_winsys.h:5:22:
error: X11/Xlib.h: No such file or directory

(This regression appears to be new today.  I'm building without a
system libX11 installed.)

Thanks,

- Chris.
-- 
Chris Ball   <cj...@la...>
One Laptop Per Child

Re: [Mesa3d-dev] some r6xx work for r600g

[Jerome G. <gl...@fr...>]

On Fri, Mar 12, 2010 at 03:05:43PM +0100, Stephan Schmid wrote:
> Hello,
> attached is a patch against rev.
> 189abd58e83af98276b9e170413918c821592f0c of Jerome Glisse's mesa repo.
> With this my RV610 runs rudimentarily (few card hangs instead of hangs
> each time of loading r600g, some rendering). The changes against r7xx
> cards are copied from the differences in r600demo.
> 
> The patch does not preserve compatibility with r7xx cards  (@ Jerome
> Glisse: Is there a way to select codepaths depending on the used chip,
> e.g. like if(rdev->chip <= RV670)... )
> Also the renderung results are somewhat different from Jerome Glisse's
> results.
> tri-flat gives the following result:
> -red background
> -there is a completely green quad rendered. Its sides are parallel to
> the window borders. Left bound is the left vertex of the desired
> triangle,  right and lower bounds are the lower right vertex of the
> triangle and upper bound is the middle of the window.
> Regards
> Stephan Schmid
> 
> 

Idea is that winsys will query pciid and set different defaultstate
function btw r600/r700 then it will call different createscreen
function btw r600/r700 (passing along pciid) and create screen
will most likely have some kind of ptr to shader function which
should pretty much the only difference from pipe pov (ignoring the
restriction stated in the open documentation for which we need the
pciid to alter the capacity of the state tracker).

Cheers,
Jerome

Re: [Mesa3d-dev] util_format cleanup leftover: Gallium BGRA vertex swizzles

[Brian P. <br...@vm...>]

Jose wrote:
> On Thu, 2010-03-11 at 17:50 -0800, Marek Olšák wrote:
> > Please see this commit:
> > http://cgit.freedesktop.org/~mareko/mesa/commit/?id=177a4432aa88fc68d83895ae71b7ad2c86a1e7e3
> >
> > Subject: [PATCH] gallium: fix BGRA vertex color swizzles
> >
> > The mapping for vertex_array_bgra:
> > (gl -> st -> translate)
> > GL_RGBA -> PIPE_FORMAT_R8G8B8A8 (RGBA) -> no swizzle (XYZW)
> > GL_BGRA -> PIPE_FORMAT_A8R8G8B8 (ARGB) -> ZYXW (BGRA again??)
> >
> > Iẗ́'s pretty clear that PIPE_FORMAT_A8R8G8B8 here is wrong. This
> > commit
> > fixes the pipe format and removes obvious workarounds in
> > util/translate.
> >
> > Tested with: softpipe, llvmpipe, r300g.
> >
> > Please review.
> 
> Marek,
> 
> Well spotted.
> 
> These were cases where formats were being used inconsistently in respect
> to their LSB->MSB vs MSB->LSB meanings and my automatic format rename
> did no more than flipping the inconsistency the otherway around.
> 
> Looks good to me.
> 
> I've went ahead and commited since I needed to fix up other state
> trackers.

Should this go into the 7.8 branch too, Jose?

-Brian

Re: [Mesa3d-dev] xdemos build breakage...

[Brian P. <br...@vm...>]

Dan Nicholson wrote:
>On Fri, Mar 12, 2010 at 6:48 AM, Jeff Smith <why...@ya...> wrote:
>>> This change:
>>>
>>> commit d888bbc45a84946cafb4f4d2c89681a580cd89bc
>>> Author: Brian Paul <br...@vm...>
>>> Date:   Tue Nov 17 13:39:13 2009 -0700
>>>
>>>    progs/xdemos: added -lX11 -lpthread for GNU gold linker
>>>
>>> breaks the build if you are building under a specific path prefix
>>> (say, /opt/XORG) and you have an existing X11 installation in the
>>> usual location (/usr, etc.)
>>
>> I found the same problem and sent a patch to this list a few hours ago to address it:
>>
>> "[PATCH] Add -L$(libdir) for xdemos and egl so that the right libX11 is found"
>
>That's not really the right thing, though. You're assuming that I have
>libX11 in the same libdir as I'm installing to and I want to use it.
>The fact is that configure uses pkg-config to check for x11 and other
>libraries needed to link the demos. It certainly was working before
>without requiring hardcoding things into the Makefiles.

Oops, I didn't see your reply, Dan.  I already committed Jeff's patch.  If you have  better fix, please revert.

-Brian

Re: [Mesa3d-dev] Mesa (gallium-sampler-view): st/mesa: Associate a sampler view with an st texture object.

[Brian P. <bri...@gm...>]

On Fri, Mar 12, 2010 at 8:23 AM, Keith Whitwell <ke...@vm...> wrote:
> On Fri, 2010-03-12 at 06:54 -0800, Luca Barbieri wrote:
>> What if you have a non-integer min LOD?
>> While the integer part may belong to the sampler view, the fractional
>> part really seems to be a sampler property.
>> Requiring min_lod < 1.0 also doesn't seem to make much sense, so
>> shouldn't it be kept as it is now?
>> Same thing for last_level / max_lod.
>
> Hmm, I see your point.  Fractional values don't have a lot of meaning in
> the views, but without a fraction from somewhere we don't capture all of
> GL semantics.
>
> I guess this is the underlying reason GL has such a wide variety of ways
> of specifying the min/max level also.
>
> And finally, it seems like DX10 has done the same thing - with both
> integer min/max in views and float min/max in the sampler state.
>
> It looks like we really do want to keep them both.

I agree.

The OpenGL GL_TEXTURE_BASE/MAX_LEVEL attributes are used to indicate
which mipmap images are actually present so that OpenGL can determine
if the texture is 'complete'.  An app might load texture LODs on
demand over time.  For example, loading level 2, then 1, then finally
0, decrementing GL_TEXTURE_BASE_LEVEL each time.

The Mesa state tracker currently always puts the OpenGL base-level
mipmap image into level[0] of the gallium texture.  I'd like to move
away from that.

The GL_TEXTURE_MIN/MAX_LOD attributes clamp the range of LODs that the
sampler accesses. These can be fractional values, as Luca said, which
is significant for linear mipmap level interpolation.

Finally, the OpenGL implementatoin should internally restrict
GL_TEXTURE_MIN/MAX_LOD to the range of levels indicated by
GL_TEXTURE_BASE/MAX_LEVEL, to avoid grabbing invalid/missing texels.

-Brian

Re: [Mesa3d-dev] Mesa (gallium-sampler-view): st/mesa: Associate a sampler view with an st texture object.

[Christoph B. <e04...@st...>]

On 12.03.2010 15:08, michal wrote:
> Keith Whitwell wrote on 2010-03-12 14:46:
>   
>> Michal,
>>
>> Is the intention to have >1 sampler view active in the Mesa state
>> tracker, specifically in the cases where min_lod varies?
>>
>> In other words, you seem to have two ways of specifying the same state:
>>
>>   pipe_sampler_view::first_level
>>
>> and
>>
>>   pipe_sampler::min_lod
>>
>> Is there a case to keep both of these?  Or is one enough?
>>
>>   
>>     
> It looks like one has to go away, and that would be 
> pipe_sampler::min_lod. And we want to have a per-texture cache of 
> sampler views in mesa.
>
>   
But there *is* a difference between
GL_TEXTURE_MIN_LOD
and
GL_TEXTURE_BASE_LEVEL
which those two seem to reflect.

How do you implement that if you just remove one ?

> ------------------------------------------------------------------------------
> Download Intel® Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Mesa3d-dev mailing list
> Mes...@li...
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
>