ping. the piglit test was pushed) Thanks, Andrii.
On Tue, Jan 22, 2019 at 1:31 PM andrey simiklit <asimiklit.w...@gmail.com> wrote: > Hello, > > Could somebody help me with a push of the following patch? > https://patchwork.freedesktop.org/patch/254397 > > This fix is needed to fix these fails: > > https://mesa-ci.01.org/global_logic/builds/56/group/ac3c5a0dc1f15492570367c6c8ec835c > > When this fix is pushed we will be able to remove the following test: > tex-upside-down-miptree > from Intel CI "[expected-crashes]" sections. > > Thanks, > Andrii. > > On Mon, Jan 14, 2019 at 3:05 PM andrey simiklit <asimiklit.w...@gmail.com> > wrote: > >> Hello, >> >> The test for this issue is pushed to the piglit. >> It would be great to push the mesa fix too if it is still an acceptable >> for all :) >> >> Thanks, >> Andrii. >> On Sat, Oct 20, 2018 at 12:29 PM andrey simiklit < >> asimiklit.w...@gmail.com> wrote: >> >>> Hello, >>> >>> On Fri, Oct 19, 2018 at 15:14 Kenneth Graunke <kenn...@whitecape.org> >>> 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 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. >>> >>> >>> 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 mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
