Hello, On Fri, Oct 19, 2018 at 15:14 Kenneth Graunke <[email protected]> wrote:
> 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 [email protected] > wrote: > > > > From: Andrii Simiklit <[email protected]> > > > > > > > > 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 <[email protected]> > > > > --- > > > > .../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 <[email protected]> > > > > 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. Thanks for reviewing :-) I will start to work on it as soon as come back from vacation (on Monday) > > --Ken Thanks, Andrii.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
