[Mesa-dev] [Bug 84566] Unify the format conversion code

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=84566

--- Comment #55 from Jason Ekstrand <jason at jlekstrand.net> ---
(In reply to Iago Toral from comment #54)
> Hi Jason,
> 
> I think we are close to completing this, so here is a summary of the current
> state and how we intend to submit the patches for review, since we assume
> that you will be reviewing most of these:
> 
> (In reply to Jason Ekstrand from comment #0)
> (...)
> > I would like to fix clean up this mess and unify a lot of the conversion
> > code.  Here's what I've envisioned:
> > 
> >  1) Autogenerate all of the color packing/unpacking functions
> 
> Done.
> 
> >  2) Convert all of the packing/unpacking functions to pack/unpack a
> > rectangle taking a width, height, and stride.  We can use 1x1 for
> > single-pixel conversions.
> 
> We decided not to do this yet because in the end format conversion will
> mostly happen through _mesa_format_convert now which already accepts a rect.
> Although making the pack/unpack functions accept a rect would slightly
> simplify _mesa_format_covert I think it won't really make a big difference
> anywhere else, so we have kept postponing this until now. We can still do
> this change if you think it is worth it.

That's fine.  It's not that big of a change.  Mostly, it's just a small
performance boost for really narrow conversions.

> >  3) Make swrast use the unpacking functions instead of its own texture
> > sampling functions.
> >  4) Add an array format enum that allows us to enumerate all possible array
> > formats.  Between mesa_format and this array format, we should also be able
> > to enumerate all of the GL datatypes.
> >  5) Make a masater conversion function that takes a void*, format, width,
> > height, and stride for both source and destination and just does the
> > conversion.  If the above mentioned array format enum is distinct from the
> > mesa_format enum, the function could be written to take a uint32_t type and
> > accept either mesa_format or an array format in the same parameter.
> >  6) Use the above master conversion function for TexSubimage, TexImage,
> > GetTexImage, DrawPixels, and ReadPixels.  We still have to deal with pixel
> > conversion, but it should vastly simplify all of them.
> 
> All this is done.
> 
> We have run piglit for i965, classic swrast, gallium swrast, llvmpipe,
> nouveau and gallium radeon without regressions so I guess it should be quite
> okay. Actually, there are now about 14 new piglit subtests that pass when
> compared to master.

Good work!  If it passes all of those, then I'm not too concerned about the
kinds of hidden bugs we hit on my first format conversion series.

> That said, I think there are probably many conversions that piglit won't
> cover, so I would not be surprised if we have a bunch of regressions anyway,
> but hopefully they won't be anything major.

I'm sure there's things piglit covers.  However, I'm going to give it a good
look over and between doing what "looks right" and passing piglit, I think
we've got it mostly covered.

> We are now preparing the patch set for review now. It is a large collection
> of patches (>50 patches), so we are thinking to split it in 3 series: one
> with general bugfixes, another for the auto-generation of pack/unpack
> functions and a final series with the master convert function.

That seems perfectly reasonable.

> Git says that the patches add ~5K LOC and remove ~15K LOC, so it a huge
> cleanup.

Nice!

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141111/22e1e719/attachment.html>