Collection Manager: Replace deprecated bgl module. Task: T69577 #104703

Merged
Member

Replace using the deprecated bgl API with the new gpu API to draw
the QCD Move widget.

Thanks to Oxicid and Germano Cavalcante (mano-wii) for helping.
#104547

Replace using the deprecated bgl API with the new gpu API to draw the QCD Move widget. Thanks to Oxicid and Germano Cavalcante (mano-wii) for helping. https://projects.blender.org/blender/blender-addons/pulls/104547
Ryan Inch added 1 commit 2023-06-19 08:40:24 +02:00
68c40c2a74 Collection Manager: Replace deprecated bgl module. Task: T69577
Replace using the deprecated bgl API with the new gpu API to draw
the QCD Move widget.

Thanks to Oxicid and Germano Cavalcante (mano-wii) for helping.
#104547
Ryan Inch requested review from Germano Cavalcante 2023-06-19 08:41:10 +02:00
Germano Cavalcante reviewed 2023-06-19 16:15:55 +02:00
@ -101,2 +96,2 @@
bgl.glEnable(bgl.GL_LINE_SMOOTH)
bgl.glHint(bgl.GL_LINE_SMOOTH_HINT, bgl.GL_NICEST)
thickness = round(1 * scale_factor())
thickness = max(thickness, 1)

The thickness value has changed. Intentional?

The thickness value has changed. Intentional?
Author
Member

It was intentional, because the gpu line shader gave different results to what I had with bgl, but on second thought I'm going to revert the outline thickness and draw order changes.

It was intentional, because the gpu line shader gave different results to what I had with bgl, but on second thought I'm going to revert the outline thickness and draw order changes.
Germano Cavalcante approved these changes 2023-06-19 16:37:59 +02:00
Germano Cavalcante left a comment
Member

Although ok, I notice some changes unrelated to removing the use of the bgl module.

But if you disregard that and that shader.bind() is no longer needed in these cases, the code looks ok.

I would double check if the drawing hasn't changed.

Although ok, I notice some changes unrelated to removing the use of the bgl module. But if you disregard that and that `shader.bind()` is no longer needed in these cases, the code looks ok. I would double check if the drawing hasn't changed.
@ -121,1 +114,3 @@
batch.draw(shader)
if not outline:
batch = batch_for_shader(shader, draw_type, {"pos": vertices})
shader.bind()

shader.bind() is no longer needed here.

`shader.bind()` is no longer needed here.
@ -122,0 +117,4 @@
shader.uniform_float("color", color)
batch.draw(shader)
else:
batch = batch_for_shader(shader, draw_type, {"pos": [(v[0], v[1], 0) for v in vertices], "color": [color for v in vertices]})

This looks like a change unrelated to the removal of the bgl module.
It should be in a separate commit.

This looks like a change unrelated to the removal of the bgl module. It should be in a separate commit.
Author
Member

No, this is related. I needed to add a separate line shader to work with the new approved methods, so this handles that.

No, this is related. I needed to add a separate line shader to work with the new approved methods, so this handles that.
@ -122,0 +118,4 @@
batch.draw(shader)
else:
batch = batch_for_shader(shader, draw_type, {"pos": [(v[0], v[1], 0) for v in vertices], "color": [color for v in vertices]})
shader.bind()

shader.bind() is no needed here.

`shader.bind()` is no needed here.
@ -138,1 +136,3 @@
batch.draw(shader)
if not outline:
batch = batch_for_shader(shader, draw_type, {"pos": vertices})
shader.bind()

shader.bind() is no longer needed here.

shader.bind() is no longer needed here.
@ -139,0 +140,4 @@
batch.draw(shader)
else:
batch = batch_for_shader(shader, draw_type, {"pos": [(v[0], v[1], 0) for v in vertices], "color": [color for v in vertices]})
shader.bind()

shader.bind() is no needed here.

shader.bind() is no needed here.
@ -265,2 +282,3 @@
batch = batch_for_shader(shader, 'LINE_STRIP', {"pos": vertices})
batch = batch_for_shader(shader, 'LINE_STRIP', {"pos": [(v[0], v[1], 0) for v in vertices], "color": [color for v in vertices]})
shader.bind()

shader.bind() is no needed here.

shader.bind() is no needed here.
Ryan Inch added 1 commit 2023-06-20 08:53:53 +02:00
Author
Member

Thanks for the review! The shader.bind calls don't seem to cause any problems and seem to be needed on previous versions of Blender (I distribute this add-on in multiple places), so it's just easier to have them. I think I could remove them for the line shader, but if I just keep them everywhere it's simpler to keep track of.

The drawing is a little different using gpu, but it's fairly close and 3.6 seems to be quite close to how it looked with bgl.

Thanks for the review! The `shader.bind` calls don't seem to cause any problems and seem to be needed on previous versions of Blender (I distribute this add-on in multiple places), so it's just easier to have them. I think I could remove them for the line shader, but if I just keep them everywhere it's simpler to keep track of. The drawing is a little different using gpu, but it's fairly close and 3.6 seems to be quite close to how it looked with bgl.
Ryan Inch merged commit be6cbe0222 into blender-v3.6-release 2023-06-20 20:58:15 +02:00
Ryan Inch deleted branch object_collection_manager-bgl-removal 2023-06-20 20:58:15 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-addons#104703
No description provided.