[Bf-committers] New glLineWidth Policy

Fri Feb 19 01:12:18 CET 2016

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
<brechtvanlommel at pandora.be> wrote:
> On Thu, Feb 18, 2016 at 11:37 PM, Mike Erwin <significant.bit at gmail.com> 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 <
>> brechtvanlommel at pandora.be> 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 <ideasman42 at gmail.com>
>>> 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 <significant.bit at gmail.com>
>>> 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 <eiseljulian at gmail.com>
>>> 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
>>> >>> Bf-committers at blender.org
>>> >>> http://lists.blender.org/mailman/listinfo/bf-committers
>>> >>>
>>> >> _______________________________________________
>>> >> Bf-committers mailing list
>>> >> Bf-committers at blender.org
>>> >> http://lists.blender.org/mailman/listinfo/bf-committers
>>> >
>>> >
>>> >
>>> > --
>>> > - Campbell
>>> > _______________________________________________
>>> > Bf-committers mailing list
>>> > Bf-committers at blender.org
>>> > http://lists.blender.org/mailman/listinfo/bf-committers
>>> _______________________________________________
>>> Bf-committers mailing list
>>> Bf-committers at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>>
>> _______________________________________________
>> Bf-committers mailing list
>> Bf-committers at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers