On Thursday, October 11, 2018 12:12:38 PM PDT Kenneth Graunke wrote: > On Thursday, October 11, 2018 11:58:40 AM PDT Kenneth Graunke wrote: > > On Tuesday, October 2, 2018 9:16:01 AM PDT asimiklit.w...@gmail.com wrote: > > > From: Andrii Simiklit <andrii.simik...@globallogic.com> > > > > > > I guess that when we calculating the width0, height0, depth0 > > > to use for function 'intel_miptree_create' we need to consider > > > the 'base level' like it is done in the > > > 'intel_miptree_create_for_teximage' > > > function. > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107987 > > > Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com> > > > --- > > > .../drivers/dri/i965/intel_tex_validate.c | 26 ++++++++++++++++++- > > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > I believe this patch is correct - we're assembling things into a new > > miptree, which we start at level 0 - so we need the sizes for level 0. > > > > Alternatively, we might be able to pass validate_first_level instead > > of 0 when calling intel_miptree_create, to make one that's only good > > up until the new base...and have to re-assemble it the next time they > > change the base. It would save memory potentially. But more copies. > > I don't have a strong preference which is better. > > > > Please do make a Piglit or dEQP test for this. > > > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > Sorry, withdrawing my review. :( Chris Forbes pointed out on IRC that > your reproducer case is backwards: > > miplevel 0 - 1x1 > miplevel 1 - 2x2 > miplevel 2 - 4x4 > > That's upside down. A proper miptree would have the base be largest: > > miplevel 0 - 4x4 > miplevel 1 - 2x2 > miplevel 2 - 1x1 > > So, yes, I could see this tripping an assert...but such a crazy texture > will never be mipmap complete. If they're expecting mipmapping, then > it seems like they should get a fallback black texture (which normally > happens for incomplete textures). If not, maybe they should get a > single miplevel? Either way, seems like we should detect insanity and > bail, rather than change size calculations for the normal sane case. >
So...looked at this again. I'm not sure why upside-down matters. At DrawArrays time, we have a single miplevel (base = 2), and are trying to put that single miplevel's image into a miptree. We do properly ignore levels 0..1 as they're beyond the base. We appear to use level 0 as the actual base, and want to store our single level at level 2. Other places (TexImage) seem to work that way too. But, we're creating the miplevel with level 0 as the base, but where level 0 has the dimensions of level 2. This doesn't work. And your patch fixes that. I tried making the actual base of the unified tree be level 2, rather than level 0...so that the BaseLevel is the actual base...but tons of things broke. So, back to Reviewed-by. I think once we get a Piglit test, I'm happy to land this patch. --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
