<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 18, 2014 at 8:32 AM, Brian Paul <span dir="ltr"><<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On 07/17/2014 12:04 PM, Jason Ekstrand wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is the first installment of some work I've been doing over the past<br>
couple of weeks to refactor mesa's texture conversion/storage code. There<br>
is more to be done and more that I have done but have not included in this<br>
series. This is the first mailing-list-ready fruits of my efforts. The<br>
important bits here include:<br>
<br>
1) Using a human-readable CSV file to describe texture formats similar to<br>
the way it is currently don in gallium. This is much easier to<br>
read/edit than the structure in formats.c. The guts of formats.c is<br>
then autogenerated from this CSV file.<br>
</blockquote>
<br></div>
I'm kind of on the fence about this. Some of us have been hoping that we'd eventually consolidate some of the Mesa and gallium code so we wouldn't have duplicated/parallel code. The format code is a good example. In theory, the MESA_FORMAT_ stuff could be replaced by the gallium PIPE_FORMAT_ code.<br>
</blockquote><div><br></div><div>Or the other way round... I intentionally kept the format of the CSV file almost identical. Hopefully, this will eventually be merged into, at the very least, one file format and parser. The only significant difference in the CSV format is that I replaced gallium's "plain" format layout with "packed" and "array" layouts. Mesa makes a distinction here that gallium does not. In Gallium, the format layout rules are basically the same as for C structure bitfields and whether a format is a packed format or an array format is determined based on the size of the channels. In mesa, the difference is expliciti and packed formats are specified in lsb-to-msb order always. We could bikeshed all day about which of these is better than the other.<br>
<br></div><div>I did make some substantial changes to the format_parser.py code from the original gallium version. Some of this was removing the difference between big and little-endian. Some was simply doing what I thought were cleanups such as adding an actual Swizzle class that allows you to compose and pseudo-invert swizzles (useful for writing conversion code).<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I kind of like the hand-written format code since it's simple to understand and update. Now you've got 600 lines of (uncommented) Python code to figure out and maintain. When something breaks it's important that the code is easily maintainable if you're not around.</blockquote>
<div><br></div><div>The problem with the hand-written format_info code is that, while adding a format is fairly easy, adding a type of information is not. The three patches that followed to remove indexBits and add packing and swizzling information would have been a nightmare. Having it laid out in a table makes this much easier. Also, if we want any sort of auto-generation, we need the table anyway. Might as well get rid of one more place where things can get out-of-sync.<br>
<br></div><div>I'll try to better comment the python code.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
2) Adding a very generic yet efficient _mesa_swizzle_and_convert function<br>
that is capable of performing the vast majority of texture format<br>
conversions in one function. It has also been fairly carefully tuned<br>
to be even faster than the _mesa_swizzle_ubyte_image special-case that<br>
we had before for ubyte textures only it also works on the other<br>
datatypes and can even do type conversions as it swizzles.<br>
<br>
3) Refactoring of texstore.c including the use of the above<br>
_mesa_swizzle_and_convert function along with the already-existing<br>
packing functions to remove a lot of hand-written special-case code.<br>
</blockquote>
<br></div>
This part generally looks good. Comments already posted.<div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks to the format CSV file, there's even more that we can now do.<br>
Things I hope to accomplish in the future include:<br>
<br>
1) Autogenerate the bulk of main/format_pack.c, main/format_unpack.c, and<br>
main/pack.c from CSV files. There's some refactoring that will be<br>
required first, but it shouldn't be that hard and I already have the<br>
python code to do the generation; it's just not part of this patch<br>
series.<br>
</blockquote>
<br></div>
Again, this kind of thing was already done for gallium. It's kind of sad to effectively duplicate it.<br>
<br>
I'm interested in hearing other opinions.<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>Agreed. However, short of replacing all of mesa's formats and format-handling code with gallium formats and handling code, we are always going to have some duplication.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888">
<br>
-Brian</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) Find a general way to do depth-stencil formats. I'm a bit dubious as<br>
to whether or not this will turn out to be practical, but I haven't had<br>
a chance to look into it too much yet.<br>
2) Do similar refactors for GetTexImage, ReadPixels, and DrawPixels.<br>
<br>
Happy Reviewing!<br>
--Jason Ekstrand<br>
</blockquote>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>