Bug 21864

Summary:	Flightgear still asserts due to RADEON_MAX_TEXTURE_LEVELS on radeon-rewrite as of today
Product:	Mesa	Reporter:	Mathias Fröhlich <Mathias.Froehlich>
Component:	Drivers/DRI/r300	Assignee:	Default DRI bug account <dri-devel>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Mathias Fröhlich 2009-05-22 00:48:04 UTC

Hi,

I have this assertion somewhere in the mipmap tree generation code.
This still happens due to the kernel header that still defines RADEON_MAX_TEXTURE_LEVELS to 12 on my system and on current linux git head.

Sorry, I have lost track of all those countless branches and git repositories that might already contain a bumped define there. So this might be 'fixed' already.

Anyway, the problem persists for the time people compile that stuff on distro kernels until at least 2.6.30 I guess.

The problem where the assertion happens is due to a static sized array in struct _radeon_mipmap_tree.
So the attached patch to the radeon-rewrite branch changes this array size to be dynamic based on the maximum number texture levels in the gl context struct.
So, this should make the driver more robust and trigger less assertions.

If this sounds good for you, please apply the atached patch to radeon-rewrite.

Thanks and greetings!

Mathias

Comment 1 Michel Dänzer 2009-05-22 02:38:54 UTC

See commit e101959b6a262ba34a12b407ea6f480e6b4d7d72 for how I fixed this in master. Looks like that wasn't properly merge into radeon-rewrite.

Comment 2 Mathias Fröhlich 2009-05-22 04:56:43 UTC

(In reply to comment #1)
> See commit e101959b6a262ba34a12b407ea6f480e6b4d7d72 for how I fixed this in
> master. Looks like that wasn't properly merge into radeon-rewrite.

Hmm, the R300_MAX_TEXTURE_SIZE define did not make it into the rewrite branch.
Indeed. The files in question are drastically changed.

I tried to define R300_MAX_TEXTURE_SIZE and use that like you did.
The problem here is that the radeon_mipmap_tree.h header where this static array size is used is a radeon/r200/r300... data structure. So using a define that is defined in R300 specific headers will not work.

Solutions:
* Define a mesa private define for the r* drivers.
* Or use a runtime dynamic sized array for that. For the rewrite branch it would be possible to use the 'array at end of struct' trick and statically declare a one sized array but CALLOC sufficient space for a glCtx->MaxTextureLevels sized array. This way the second malloc could be avoided - if this is your concern.

BTW. tell me if I am just too impatient. I hope that I can help to push that corners where I can see problems. But if that is too early, tell me ...
Willing to provide patches when helpful.

Greetings and Thanks

Mathias

Comment 3 Dave Airlie 2009-06-11 18:41:48 UTC

can you test with mesa master? (I merged rewrite and fixed this afterwards.)

Comment 4 Mathias Fröhlich 2009-06-22 22:05:14 UTC

(In reply to comment #3)
> can you test with mesa master? (I merged rewrite and fixed this afterwards.)
Works now.
Marked this bug as fixed.
Thanks!

Mathias

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.