Here's the latest report by the way: https://developer.blender.org/T47454
On Fri, Feb 19, 2016 at 1:09 AM, Brecht Van Lommel <[email protected]> wrote: > On Thu, Feb 18, 2016 at 11:37 PM, Mike Erwin <[email protected]> > wrote: >> For 2.77 what is left to fix? If it's a small set I say fix those and keep >> the change. (but of course I would say that) It gets us closer to clean GL >> usage. > > But it also breaks addon backwards compatibility, which I don't think > is intentional for 2.77? > > Up to now there have been multiple fixes based on bug reports from > users. If we rely on just bug reports then there's likely more corner > cases that are still broken. If we keep these commits then I think > someone should go over all GL_LINE drawing and verify that it's > properly setting the line widths. > >> On an admin / phabricator note, I've never gotten emails about the concern >> raised or new posts on that concern. Severin told me about it on IRC. Is >> there some button or setting I overlooked? > > Not sure, you could check the email preferences in your account settings? > >> Mike Erwin >> musician, naturalist, pixel pusher, hacker extraordinaire >> >> On Thu, Feb 18, 2016 at 4:34 PM, Brecht Van Lommel < >> [email protected]> wrote: >> >>> I replied to the commit, but basically my opinion is that if we can't >>> be reasonably sure that we can fix all issues before 2.77, then we >>> should just revert these commits now. >>> >>> I think explicitly specifying the line width is fine, but then let's >>> design a line drawing API such that you can't possibly make the >>> mistake of forgetting to specify the line width, by making it a >>> parameter to the line drawing API function(s). As long as we are >>> refactoring OpenGL code I prefer code to follow the convention that >>> OpenGL state should always revert to the default state as specified in >>> GPU_state_init(), specifically because it's difficult to understand >>> and track all the hidden assumptions in the existing code. Even if we >>> fix all cases in Blender we might break addons that do custom OpenGL >>> drawing. >>> >>> I think that's a general principle we should follow, not just for line >>> widths but for most OpenGL state. Think about line stipple, line >>> smooth, do we also want to disable those everywhere before drawing >>> lines? We are already assuming OpenGL is in the default state for most >>> things, I don't think line width should be the exception. >>> >>> >>> On Thu, Feb 18, 2016 at 9:44 PM, Campbell Barton <[email protected]> >>> wrote: >>> > Hi, think its OK to keep this policy change. >>> > >>> > Even though its been a bit of a hassle to clear up all the >>> > line-width's the past few weeks. >>> > Before this - we would run into occasional bugs where the state wasn't >>> > properly reset. >>> > Finding out which function left the state unset could be quite >>> > involved ... especially with a lot of nested calls in the drawing >>> > code, where its not always clear at what point the state should be >>> > reset. >>> > >>> > Even though the number of calls to glLineWidth may be higher, >>> > (since previous code assumed width of 1 everywhere), >>> > the driver *should* check if the width is the same as the previous, >>> > and exit-early >>> > (verified with Mesa and NVidia's drivers). >>> > >>> > On Fri, Feb 19, 2016 at 6:32 AM, Mike Erwin <[email protected]> >>> wrote: >>> >> Hey Julian, thanks for bringing this up for discussion. The intentions >>> of >>> >> that recent change were twofold, and you summed up the difference >>> between >>> >> the new and old policy perfectly. >>> >> >>> >> First intention was to reduce GL state changes -- which you say might >>> not >>> >> be reduced. And more importantly to make each bit of line-drawing code >>> >> responsible for its own line width. To me it's way more paranoid for >>> each >>> >> site to reset it back to 1.0 just in case some other unrelated draw call >>> >> forgets to set its own line width. Specifying state at the draw call >>> site >>> >> is also more Vulkan-friendly, whenever we get to that. >>> >> >>> >> I did a similar change set with point size a week or two before and it >>> >> seemed to work really well. Lines are drawn in many more places (and >>> more >>> >> ways) so some things were not caught and updated to the new policy. >>> >> >>> >> So yes, let's review whether or not the new policy is a good one. >>> Naturally >>> >> I'm on Team Yes! >>> >> >>> >> Mike Erwin >>> >> musician, naturalist, pixel pusher, hacker extraordinaire >>> >> >>> >> On Thu, Feb 18, 2016 at 1:53 PM, Julian Eisel <[email protected]> >>> wrote: >>> >> >>> >>> Hey there, >>> >>> >>> >>> We talked about this in IRC before, but I'd like to get a final >>> >>> conclusion on this topic and give other devs the chance to raise their >>> >>> voice. >>> >>> >>> >>> In a recent commit [1] we changed our policy for glLineWidth calls. >>> >>> Previously, convention was to always reset to default (1.0) after >>> >>> drawing with non-default width. New convention is to always ensure >>> >>> line width is set to correct value before drawing lines (so there >>> >>> basically is no default state anymore). >>> >>> >>> >>> The changed policy caused quite a few regressions, which we can fix of >>> >>> course - issue is finding them. The main reasons I'm not sure about >>> >>> this however, are in the following: >>> >>> Although it might look like state changes were reduced, I'm afraid >>> >>> we'll end up with more state changes than before, since we do much >>> >>> more line draw calls than glLineWidth calls to change to non-default >>> >>> (>400 vs <100). Campbell made the point that we shouldn't need to be >>> >>> too paranoid, adding glLineWidth all over, but I guess a fair amount >>> >>> of paranoia would actually be needed since it's often hard to predict >>> >>> all possible previous line-width states. And not to forget, when >>> >>> adding code with changed line-width you also had to check possible >>> >>> following draw calls. >>> >>> Another point Campbell made, is that old policy would be hard to >>> >>> follow for nested draw calls. Although I agree it's a valid point, we >>> >>> managed to do it all the time, so it's obviously doable (Sergey added >>> >>> we should avoid nested draw calls anyway). >>> >>> >>> >>> So IMHO, this is too much hassle, and output isn't guaranteed to be >>> worth >>> >>> it... >>> >>> All that said, I could live with the new convention of course, but >>> >>> let's review it first again :) >>> >>> >>> >>> [1] https://developer.blender.org/rBSe25ba162c0b >>> >>> >>> >>> Cheers, >>> >>> - Julian - >>> >>> _______________________________________________ >>> >>> Bf-committers mailing list >>> >>> [email protected] >>> >>> http://lists.blender.org/mailman/listinfo/bf-committers >>> >>> >>> >> _______________________________________________ >>> >> Bf-committers mailing list >>> >> [email protected] >>> >> http://lists.blender.org/mailman/listinfo/bf-committers >>> > >>> > >>> > >>> > -- >>> > - Campbell >>> > _______________________________________________ >>> > Bf-committers mailing list >>> > [email protected] >>> > http://lists.blender.org/mailman/listinfo/bf-committers >>> _______________________________________________ >>> Bf-committers mailing list >>> [email protected] >>> http://lists.blender.org/mailman/listinfo/bf-committers >>> >> _______________________________________________ >> Bf-committers mailing list >> [email protected] >> http://lists.blender.org/mailman/listinfo/bf-committers _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
