[Mesa-dev] MapTextureImage patch series for review

[Eric Anholt]

On Tue, 16 Aug 2011 18:53:00 -0600, Brian Paul <brianp at vmware.com> wrote:
> On 08/15/2011 12:53 PM, Eric Anholt wrote:
> > Here's my "mti-tested" branch of the map-texture-image-v4 work.  It's
> > not regressing on intel or softpipe, and in fact fixes a couple of
> > tests now on those.  It is not as complete a rework as
> > map-texture-image-v4, but it's quite a bit of the change and it it
> > should be bisectable (I've had to several times so far to work out the
> > regressions in map-texture-image-v4).
> 
> Hmmm, yeah, a lot of the core/swrast refactoring I did isn't there.  I 
> guess I can redo that on top of your patch series, but it'll take some 
> time (and I've had virtually no spare time lately).

Note that for a bunch of the driver typing for the swrast base-classing,
coccinelle was *very* useful for producing clean changes:

@@
@@
struct intel_texture_image {
- struct gl_texture_image base;
+ struct swrast_texture_image base;
  ...
};

@@
struct intel_texture_image *I;
@@
- I->base
+ I->base.Base

(though I think something that noticed that there was a gl_texture_image
that the intel_texture_image came from and dereffing that instead would
be a lot nicer).

With that, I found the rebasing of the rest of your branch on top a lot
less painful when I was doing it.  The issue I saw was how swrast's
private fields would get updated when a teximage is initialized, but
that should be a matter of a small init function in swrast.

> > However, despite the two driver issues, I'd still like to push the
> > code.  I think it makes maintenance of classic drivers much easier.
> > Before doing so, I'd like to get at last an ack from Brian and the
> > other driver maintainers for the changes to their stuff.  If you don't
> > want to review/ack the whole series, just mention it for the driver
> > patch in question.
> 
> I'd like to see this merged/pushed soon too.  However, we've got a 
> problem with the gallium drivers.  The non-scons build complains that 
> _mesa_meta_init() isn't defined because meta.c isn't included in the 
> libmesagallium.a library.  I had a patch for this, but it was pretty 
> ugly (and I think I lost it).
>
> The problem with meta.c is it uses swrast code (which we don't want in 
> gallium builds - though, that's what the SCons build currently does).

Ouch.  Good point.  What I'm confused by is how I'm sure I managed to
build and test softpipe despite this.

I found that the gallium decompression produced regressions when it
started getting used for mipmap generation, which was why I started
using the meta there.

> But the only thing in meta.c that gallium uses (indirectly) is the new 
> texture decompression code.

I've been thinking about trying to pull the glGetTexImage meta
decompression out and get the current set of changes in minus that.  I
don't think it's quite baked yet -- it still has issues with snorm
compressed textures, and I fear it doesn't support all the format/type
combos that would be valid for glGetTexImage. I really want the
generatemipmaps cleanup, though, so I'm not sure how to handle getting
the fetch functions what they need if we were to go that way for now.

However, if we can confirm that the format/types stuff for glGetTexImage
of compressed is good (we don't have tests for it), I'd be up for making
snorm decompression rely on snorm RTT and go ahead with the metaops.  We
don't support that snorm RTT in i965 right now because of span functions
despair.  I'd like to see a renderbuffer mapping hook like this and
avoid spans functions before flailing around more with that.  If
everything works out, we're hoping to sort that out before the next
release.

> One fix would be to refactor meta.c into several files to separate out 
> the meta functions that use swrast.

I typed out basically that sentence before coming across this
paragraph.  I agree!

> Another approach is to add more functions like 
> _mesa_meta_check_generate_mipmap_fallback() to query if the meta 
> function can do its job, then remove all the swrast calls and leave it 
> up to the caller to call the meta or swrast functions.

I've been wanting less of that.  At least for mipmaps generation, the
driver hooks all ended up being

if (_mesa_meta_check_generate_mipmap_fallback())
   _mesa_generate_mipmaps();
else
   _mesa_meta_generate_mipmaps().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110817/7dcd074c/attachment.pgp>