Re: R300 cleanup questions
Keith Whitwell wrote: Oliver McFadden wrote: I'd like some input on the VBO stuff in r300. In r300_context.h we have the following. /* KW: Disable this code. Driver should hook into vbo module * directly, see i965 driver for example. */ /* #define RADEON_VTXFMT_A */ #ifdef RADEON_VTXFMT_A #define HW_VBOS #endif So the VTXFMT (radeon_vtxfmt_a.c) code is disabled anyway. This also disables hardware VBOs. I guess this has been done since the new VBO branch was merged. So, the question is, should this dead code be removed? I think all drivers are (or should be) moving to the new VBO code anyway. I've already made a patch for this, but I'm not committing until I get the okay from a few people. Yes, the old code should go. I guess there might be some starting points in there for beginning the vbo work, that's about the only reason to keep it. Hmm, I just took a look through the r300 code, and was surprised to see myself listed as the author of several of the files?? I'm pretty sure I haven't done any work on that driver... I think I'd prefer to have a line that says based on xxx by Keith Whitwell, or even just remove my name from the r300_* files and give credit instead to the people who've really been working on that code... Keith - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol export and
other things like that.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr
Why move all the comments to the head of the file. IMO the method should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet */
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar which
i dislike but the choice is up to the first person who do big cleanup :) and
we do not have to conform to mesa coding style for driver but use the one
we like the more.
best,
Jerome Glisse
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Jerome Glisse wrote:
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol export
and
other things like that.
Yes, make functions static whenever possible.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr
Why move all the comments to the head of the file. IMO the method should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet */
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar which
i dislike but the choice is up to the first person who do big cleanup :) and
we do not have to conform to mesa coding style for driver but use the one
we like the more.
Yes, core Mesa has a fairly consistant naming scheme but it's the
prerogative of the driver writers to choose their style. That said,
naming within each driver should be consistant.
-Brian
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Hi,
I added the not implemented yet comment back, although there are other places
that use 65535 so it could be some kind of hardware limit...
The only reason that I went with camel case r300FooBar names is because that's
what 90% of the driver uses; it's easier to change a few r300_foo_bar to
r300FooBar than the other way around. The important thing is it's consistent.
Now I just hope I don't get shot for all the commits. ;)
On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote:
Jerome Glisse wrote:
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol
export and
other things like that.
Yes, make functions static whenever possible.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr
Why move all the comments to the head of the file. IMO the method should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet */
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar which
i dislike but the choice is up to the first person who do big cleanup :)
and
we do not have to conform to mesa coding style for driver but use the one
we like the more.
Yes, core Mesa has a fairly consistant naming scheme but it's the
prerogative of the driver writers to choose their style. That said,
naming within each driver should be consistant.
-Brian
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
I also think we might need to add _dri_warning/_dri_error because the _mesa
versions output Mesa warning: %s which implies to the user this is a Mesa
problem, not a DRI driver problem.
I could add r300Warning and r300Error, but probably all DRI drivers need warning
and error functions... So maybe adding them to the common DRI code?
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
Hi,
I added the not implemented yet comment back, although there are other
places
that use 65535 so it could be some kind of hardware limit...
The only reason that I went with camel case r300FooBar names is because
that's
what 90% of the driver uses; it's easier to change a few r300_foo_bar to
r300FooBar than the other way around. The important thing is it's
consistent.
Now I just hope I don't get shot for all the commits. ;)
On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote:
Jerome Glisse wrote:
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol
export and
other things like that.
Yes, make functions static whenever possible.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr
Why move all the comments to the head of the file. IMO the method
should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet
*/
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar which
i dislike but the choice is up to the first person who do big cleanup :)
and
we do not have to conform to mesa coding style for driver but use the
one
we like the more.
Yes, core Mesa has a fairly consistant naming scheme but it's the
prerogative of the driver writers to choose their style. That said,
naming within each driver should be consistant.
-Brian
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
I'd like some input on the VBO stuff in r300. In r300_context.h we have the
following.
/* KW: Disable this code. Driver should hook into vbo module
* directly, see i965 driver for example.
*/
/* #define RADEON_VTXFMT_A */
#ifdef RADEON_VTXFMT_A
#define HW_VBOS
#endif
So the VTXFMT (radeon_vtxfmt_a.c) code is disabled anyway. This also disables
hardware VBOs. I guess this has been done since the new VBO branch was merged.
So, the question is, should this dead code be removed? I think all drivers are
(or should be) moving to the new VBO code anyway.
I've already made a patch for this, but I'm not committing until I get the okay
from a few people.
Thanks.
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
I also think we might need to add _dri_warning/_dri_error because the _mesa
versions output Mesa warning: %s which implies to the user this is a Mesa
problem, not a DRI driver problem.
I could add r300Warning and r300Error, but probably all DRI drivers need
warning
and error functions... So maybe adding them to the common DRI code?
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
Hi,
I added the not implemented yet comment back, although there are other
places
that use 65535 so it could be some kind of hardware limit...
The only reason that I went with camel case r300FooBar names is because
that's
what 90% of the driver uses; it's easier to change a few r300_foo_bar to
r300FooBar than the other way around. The important thing is it's
consistent.
Now I just hope I don't get shot for all the commits. ;)
On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote:
Jerome Glisse wrote:
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int
prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int
prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol
export and
other things like that.
Yes, make functions static whenever possible.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long
addr
Why move all the comments to the head of the file. IMO the method
should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet
*/
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the
limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar
which
i dislike but the choice is up to the first person who do big cleanup
:)
and
we do not have to conform to mesa coding style for driver but use the
one
we like the more.
Yes, core Mesa has a fairly consistant naming scheme but it's the
prerogative of the driver writers to choose their style. That said,
naming within each driver should be consistant.
-Brian
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Here is the patch.
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
I'd like some input on the VBO stuff in r300. In r300_context.h we have the
following.
/* KW: Disable this code. Driver should hook into vbo module
* directly, see i965 driver for example.
*/
/* #define RADEON_VTXFMT_A */
#ifdef RADEON_VTXFMT_A
#define HW_VBOS
#endif
So the VTXFMT (radeon_vtxfmt_a.c) code is disabled anyway. This also
disables
hardware VBOs. I guess this has been done since the new VBO branch was
merged.
So, the question is, should this dead code be removed? I think all drivers
are
(or should be) moving to the new VBO code anyway.
I've already made a patch for this, but I'm not committing until I get the
okay
from a few people.
Thanks.
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
I also think we might need to add _dri_warning/_dri_error because the
_mesa
versions output Mesa warning: %s which implies to the user this is a
Mesa
problem, not a DRI driver problem.
I could add r300Warning and r300Error, but probably all DRI drivers need
warning
and error functions... So maybe adding them to the common DRI code?
On 5/9/07, Oliver McFadden [EMAIL PROTECTED] wrote:
Hi,
I added the not implemented yet comment back, although there are other
places
that use 65535 so it could be some kind of hardware limit...
The only reason that I went with camel case r300FooBar names is
because
that's
what 90% of the driver uses; it's easier to change a few r300_foo_bar to
r300FooBar than the other way around. The important thing is it's
consistent.
Now I just hope I don't get shot for all the commits. ;)
On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote:
Jerome Glisse wrote:
On 5/8/07, Christoph Brill [EMAIL PROTECTED] wrote:
I reviewed the cleanup done by Olliver McFadden and had the
following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int
prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int
prim)
Is it necessary/usefull that the function is static?
I think it's better to have static function, i am thinking of symbol
export and
other things like that.
Yes, make functions static whenever possible.
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long
addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long
addr
Why move all the comments to the head of the file. IMO the method
should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented
yet
*/
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a
hardware
limitation while the limitation can be worked around or the
limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
We can use the one we like, i prefer r300_foo_bar over r300FooBar
which
i dislike but the choice is up to the first person who do big
cleanup
:)
and
we do not have to conform to mesa coding style for driver but use
the
one
we like the more.
Yes, core Mesa has a fairly consistant naming scheme but it's the
prerogative of the driver writers to choose their style. That said,
naming within each driver should be consistant.
-Brian
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
From 89e03351b354231117240f34bf7cc48c76a3e99b Mon Sep 17 00:00:00 2001
From: Oliver McFadden [EMAIL PROTECTED]
Date: Wed, 9 May 2007 15:50:11 +
Subject: [PATCH] r300: Removed the deprecated VTXFMT code.
---
src/mesa/drivers/dri/r300/Makefile |1 -
src/mesa/drivers/dri/r300/r300_context.c| 10 +-
src/mesa/drivers/dri/r300/r300_context.h| 19 -
src/mesa/drivers/dri/r300/r300_ioctl.c |8 -
src/mesa/drivers/dri/r300/r300_maos.c | 26 -
src/mesa/drivers/dri/r300/r300_render.c | 44 ++
src/mesa/drivers/dri/r300/r300_state.c |4 -
src/mesa/drivers/dri/r300/radeon_vtxfmt_a.c | 662 ---
8 files changed, 45 insertions(+), 729 deletions(-)
delete mode 100644 src/mesa/drivers/dri/r300/radeon_vtxfmt_a.c
diff --git a/src/mesa/drivers/dri/r300/Makefile
b/src/mesa/drivers/dri/r300/Makefile
index
Re: R300 cleanup questions
Oliver McFadden wrote: I'd like some input on the VBO stuff in r300. In r300_context.h we have the following. /* KW: Disable this code. Driver should hook into vbo module * directly, see i965 driver for example. */ /* #define RADEON_VTXFMT_A */ #ifdef RADEON_VTXFMT_A #define HW_VBOS #endif So the VTXFMT (radeon_vtxfmt_a.c) code is disabled anyway. This also disables hardware VBOs. I guess this has been done since the new VBO branch was merged. So, the question is, should this dead code be removed? I think all drivers are (or should be) moving to the new VBO code anyway. I've already made a patch for this, but I'm not committing until I get the okay from a few people. Yes, the old code should go. I guess there might be some starting points in there for beginning the vbo work, that's about the only reason to keep it. Keith - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Oliver McFadden wrote: I'd like some input on the VBO stuff in r300. In r300_context.h we have the following. /* KW: Disable this code. Driver should hook into vbo module * directly, see i965 driver for example. */ /* #define RADEON_VTXFMT_A */ #ifdef RADEON_VTXFMT_A #define HW_VBOS #endif So the VTXFMT (radeon_vtxfmt_a.c) code is disabled anyway. This also disables hardware VBOs. I guess this has been done since the new VBO branch was merged. So, the question is, should this dead code be removed? I think all drivers are (or should be) moving to the new VBO code anyway. I've already made a patch for this, but I'm not committing until I get the okay from a few people. I'm no expert on the R300 code so I'll defer. Keith might have some comments but he's very busy with another project ATM. On thing: I'd like to keep the trunk/master code stable for the upcoming 7.0 release. If you think there's some risk, let me first make the 7.0/stable branch in git. -Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Oliver McFadden wrote: I also think we might need to add _dri_warning/_dri_error because the _mesa versions output Mesa warning: %s which implies to the user this is a Mesa problem, not a DRI driver problem. I could add r300Warning and r300Error, but probably all DRI drivers need warning and error functions... So maybe adding them to the common DRI code? You could just stick with _mesa_warning() and prefix the messages with R300. -Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Well both Keith and Jerome are okay with me removing the VTXFMT code, so I'll go ahead and do that. I don't think there is any serious risk as I'm only removing code that is already disabled. :) Brian, let me know if you want to make a branch so I know when I can push. On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote: Oliver McFadden wrote: I also think we might need to add _dri_warning/_dri_error because the _mesa versions output Mesa warning: %s which implies to the user this is a Mesa problem, not a DRI driver problem. I could add r300Warning and r300Error, but probably all DRI drivers need warning and error functions... So maybe adding them to the common DRI code? You could just stick with _mesa_warning() and prefix the messages with R300. -Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
If it's just dead code removal, go ahead. -Brian Oliver McFadden wrote: Well both Keith and Jerome are okay with me removing the VTXFMT code, so I'll go ahead and do that. I don't think there is any serious risk as I'm only removing code that is already disabled. :) Brian, let me know if you want to make a branch so I know when I can push. On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote: Oliver McFadden wrote: I also think we might need to add _dri_warning/_dri_error because the _mesa versions output Mesa warning: %s which implies to the user this is a Mesa problem, not a DRI driver problem. I could add r300Warning and r300Error, but probably all DRI drivers need warning and error functions... So maybe adding them to the common DRI code? You could just stick with _mesa_warning() and prefix the messages with R300. -Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: R300 cleanup questions
Done. On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote: If it's just dead code removal, go ahead. -Brian Oliver McFadden wrote: Well both Keith and Jerome are okay with me removing the VTXFMT code, so I'll go ahead and do that. I don't think there is any serious risk as I'm only removing code that is already disabled. :) Brian, let me know if you want to make a branch so I know when I can push. On 5/9/07, Brian Paul [EMAIL PROTECTED] wrote: Oliver McFadden wrote: I also think we might need to add _dri_warning/_dri_error because the _mesa versions output Mesa warning: %s which implies to the user this is a Mesa problem, not a DRI driver problem. I could add r300Warning and r300Error, but probably all DRI drivers need warning and error functions... So maybe adding them to the common DRI code? You could just stick with _mesa_warning() and prefix the messages with R300. -Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
R300 cleanup questions
I reviewed the cleanup done by Olliver McFadden and had the following
questions:
-int r300_get_num_verts(r300ContextPtr rmesa, int num_verts, int prim)
+static int r300NumVerts(r300ContextPtr rmesa, int num_verts, int prim)
Is it necessary/usefull that the function is static?
-/* Immediate implementation has been removed from CVS. */
-
-/* vertex buffer implementation */
-
-static void inline fire_EB(r300ContextPtr rmesa, unsigned long addr
+static void inline r300FireEB(r300ContextPtr rmesa, unsigned long addr
Why move all the comments to the head of the file. IMO the method should
have a doxygen comment that states it is the vertex buffer
implementation of fire_EB, right?
-if (num_verts 65535) { /* not implemented yet */
+if (num_verts 65535) {
Comments like this should be kept. Otherwise it looks like a hardware
limitation while the limitation can be worked around or the limitation
does not exist.
Last but not least is
r300_foo_bar
preferred or
r300FooBar
Which is the one mesa uses?
Otherwise thumbs up for starting to cleanup code... on of the most
unthankfull jobs :-)
Regards,
Christoph Brill
-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
