Bugzilla – Bug 6412
mach64 vertex buffer cleanup
Last modified: 2009-08-24 12:23:46 UTC
mach64 is unique is having two implementations of VB, one that uses the
"template" vertex format and one that uses the "native" vertex format.
The "template" implementation uses the mesa provided t_dd_vb.c. However,
it still provides its own version of t_dd_vbtmp.h (mach64_vbtmp.h) and
adds complex draw functions in mach64_tris.c for conversion to native format.
The "native" implementation has simple draw functions. However, it performs
conversion to native format during emit (mach64_native_vbtmp.h) and back to
template format in translate (mach64_native_vb.c), these functions differ
greatly from the tnl_dd templates.
The attached patches have yet another implementation of the mach64 VB.
They define mach64Vertex as a struct that matches the native vertex format,
this allows to use both the mesa templates with very little changes and
the simple draw functions.
The quirks are:
* we still have to provide slightly modified versions of templates t_dd_vb.c
and t_dd_vbtmp.h, however the changes to t_dd_vb.c are minimal and can be
dropped with another iteration of cleanups if the patches are accepted.
* they drop MACH64_PREMULT_TEXCOORDS, it did not seem to make a difference
* they push conversion to little-endian down to the draw functions, these
changes are untested as I have x86.
Created attachment 5096 [details] [review]
patch for mach64 DRI
Created attachment 5097 [details] [review]
t_dd_vb.c for mach64
Created attachment 5098 [details] [review]
t_dd_vbtmp.h for mach64
I haven't looked at your patches yet ... You may want to take a look at the
t_vertex.c based code in e.g. the Savage driver. Template-based vertex setup
code in some drivers has been replaced with the t_vertex-based code. If it is
flexible enough to generate mach64 native vertices that may be beneficial in
terms of binary and source code size, though not necessarily performance.
Created attachment 5100 [details] [review]
big patch for mach64 DRI
This patch does not add files, instead it merges the changes to existing files.
Specifically, it incorporates the changes for NATIVE_VTXFMT in mach64_vbtmp.h
which would only be used for the template format. mach64_vbtmp.h was derived
from t_dd_vbtmp.h v3.5 while the changes for native format are derived
from t_dd_vbtmp.h v5.0.1 (current). Thus the changes for mach64_vbtmp.h are
those between the different versions of t_dd_vbtmp.h and the additions for the
native format (patch 5098).
As a side effect, it fixes the mach64 driver when compiled with the template
format (currently it shows a black screen).
(In reply to comment #4)
> I haven't looked at your patches yet ... You may want to take a look at the
> t_vertex.c based code in e.g. the Savage driver. Template-based vertex setup
> code in some drivers has been replaced with the t_vertex-based code. If it is
> flexible enough to generate mach64 native vertices that may be beneficial in
> terms of binary and source code size, though not necessarily performance.
I'll have a look at it,
thanks for the pointer.
Created attachment 5106 [details] [review]
big patch for mach64 DRI - try 2
this version of the patch:
* finishes the changes for LE32 (which are untested because of I have x86)
* drops the xy variables in the draw functions and uses symbolic names v.x, v.y
Other than bug fixes, I do not plan to do other changes in the near future.
Created attachment 5109 [details] [review]
patch to tnl_dd_vb.c
This is just some casts to GLfloat in print_vertex.
Created attachment 5126 [details] [review]
big patch for mach64 DRI - try 3
merge some codepaths for "native" and "template".
I looked at t_vertex.c, it seems to use extensively byte offsets from the
template vertex format which does not fit very well with the mach64 native
vertex format. If we were to use it, we would also have to use the "complex"
draw functions in mach64_tris.c
Created attachment 5155 [details] [review]
big patch for mach64 DRI - try 4
fix C version of DO_COPY_VERTEX to work on LE, by inspection it should also
work on BE.
don't drop the PREMUL_TEXCOORDS code, just disable by default since it seems to
marginally hurt performance.
Created attachment 5164 [details] [review]
big patch for mach64 DRI - try 5
Set pv.q1 correctly, fixes regression for tests/projtex.
Enable PREMULT_TEXCOORDS by default, rumored to be for multitexturing.
Created attachment 5198 [details] [review]
use t_vertex.c - try 1
Leave native vertex format implementation in peace. Port the template vertex
format implementation over to tnl from tnl_dd.
Created attachment 5206 [details] [review]
use t_vertex.c - try 2
this is pretty much as far as I can go without additions to t_vertex.c
Created attachment 5209 [details] [review]
use t_vertex.c - try 3
quasi-native vertex format.
we use the following hack in attribute emission:
/* The TNL module requires that the first attribute is _TNL_ATTRIB_POS,
* at the same time, the mach64 native vertex format has the XYZW coords
* at non-consecutive positions spread inside the vertex.
* We reserve space for the XYZW coords with EMIT_PAD's and when the vertex
* is copied to the vertex buffer for submission to the card, we use the
* _TNL_ATTRIB_POS values to fill in the pads reserved for XYZW and discard
* the first 4 bytes of the vertex.
Created attachment 5282 [details] [review]
use t_vertex.c - try 4
port to new RENDERINPUTS macros
Created attachment 5723 [details] [review]
use t_vertex.c - try 5
For the native vertex format (default and faster), this patch only touches
mach64FastRenderClippedPoly() to fix a problem I'm currently seeing with the
For the template vertex format (optional and slower), it replaces the current
non-working code with code based on t_vertex.c. It kills more than 1KLoC. It is
untested on BE. So far, I have only seen the following regression with regard
to the native format: in tests/texgenmix, the first texture with 3 coords is
not drawn the first time it is mapped.
Anyboby cares to test/review ?
This bug has served well in understanding mach64 vertex setup but I don't think
it is clean enough to apply.
Possible outcomes of this bug report:
* mach64 has to be compiled with -fno-strict-aliasing to run ok. The culprit
for this is mach64FastRenderClippedPoly() in mach64_tris.c:1580. Either
drop the optimized version of that function or see patch for an alternative.
* The non-native vertex buffer does not currently work and I don't know how
well it serves as a source of documentation. It could propably be dropped.
Mass version move, cvs -> git