Fix #106641: missing Annotation widgets #112688

Milan Davidović · 2023-09-21T21:03:06+02:00

Milan Davidović commented

2023-09-21 21:03:06 +02:00

Annotation layer widget was missing on the Tool Settings bar and the
side bar for some of the editors, as well as both color and layer
widgets on the Tool tab of the Properties editor.

Also fixed the Tool Settings bar not updating when drawing annotations
on the Image editor viewport, and the Tool panel in Properties editor
not updating when drawing annotations on the 3D scene.

Before above, after below:

Annotation layer widget was missing on the Tool Settings bar and the side bar for some of the editors, as well as both color and layer widgets on the Tool tab of the Properties editor. Also fixed the Tool Settings bar not updating when drawing annotations on the Image editor viewport, and the Tool panel in Properties editor not updating when drawing annotations on the 3D scene. --- Before above, after below: ![image](/attachments/9b22fe4b-5d96-4fba-b020-cda289b69208)

image.png

208 KiB

Milan Davidović added 1 commit 2023-09-21 21:03:20 +02:00

b7faa5a4ac Fix #106641 : missing Annotation widgets

Annotation layer widget was missing on the Tool Settings bar and the
side bar for some of the editors, as well as both color and layer
widgets on the Tool tab of the Properties editor.

Also fixed the Tool Settings bar not updating when drawing annotations
on the Image editor viewport, and the Tool panel in Properties editor
not updating when drawing annotations on the 3D scene.

Milan Davidović requested review from Pratik Borhade 2024-02-04 18:26:50 +01:00

Pratik Borhade reviewed 2024-02-05 12:20:33 +01:00

Pratik Borhade left a comment

Hi, thanks for the PR. Works correctly here. Added one comment.

Haven't checked the every case of space listener code but looks correct to me on first look.

@pablovazquez , can you review as well? 🙂

This PR is been around for ~5 months without review/reviewer. Sorry for that.

Hi, thanks for the PR. Works correctly here. Added one comment. Haven't checked the every case of space listener code but looks correct to me on first look. @pablovazquez , can you review as well? 🙂 - - - This PR is been around for ~5 months without review/reviewer. Sorry for that.

scripts/startup/bl_ui/space_toolsystem_toolbar.py Outdated

						
				@ -187,0 +188,4 @@

				                    row = layout.row(align=True)

				                    row.prop(gpl, "color", text="Annotation")

				                    row.popover(

				                        panel="PROPERTIES_PT_annotation_layers",

Pratik Borhade commented

2024-02-05 12:16:46 +01:00

why not to use TOPBAR_PT_annotation_layers here?

why not to use `TOPBAR_PT_annotation_layers` here?

Milan Davidović commented

2024-02-05 21:57:30 +01:00

If I remember correctly, TOPBAR_PT_annotation_layers didn't work for Tools tab in Properties when I was first fixing this. That's why I added PROPERTIES_PT_annotation_layers, the only difference being bl_region_type = 'WINDOW' instead of 'HEADER'. But now it works with TOPBAR_PT_annotation_layers when applied to blender-v4.0-release.

Should I change it to TOPBAR_PT_annotation_layers? The name doesn't really reflect the usage context though.

If I remember correctly, `TOPBAR_PT_annotation_layers` didn't work for Tools tab in Properties when I was first fixing this. That's why I added `PROPERTIES_PT_annotation_layers`, the only difference being `bl_region_type = 'WINDOW`' instead of `'HEADER'`. But now it works with `TOPBAR_PT_annotation_layers `when applied to blender-v4.0-release. Should I change it to `TOPBAR_PT_annotation_layers`? The name doesn't really reflect the usage context though.

Pratik Borhade commented

2024-02-06 11:35:04 +01:00

Yes, I would prefer to use TOPBAR_PT_annotation_layers :)

Yes, I would prefer to use `TOPBAR_PT_annotation_layers` :)

Milan Davidović commented

2024-02-06 13:40:08 +01:00

Sure, I'll make the change. :)

MilanDavidovic marked this conversation as resolved

Pratik Borhade requested review from Pablo Vazquez 2024-02-06 11:38:35 +01:00

Milan Davidović added 1 commit 2024-02-06 15:26:27 +01:00

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

365efa81fd Fix #106641 : missing Annotation widgets

Reuse TOPBAR_PT_annotation_layers for annotation layer popup in the Tool tab of the Properties panel.

Pratik Borhade approved these changes 2024-02-07 10:52:54 +01:00

Pratik Borhade left a comment

Thanks 🙏

👍 1

Pratik Borhade requested review from Harley Acheson 2024-03-21 11:20:04 +01:00

Harley Acheson commented

2024-03-22 17:29:55 +01:00

I updated the first comment to include a capture that illustrates some of the changes here.

Accepting, but I think @pablovazquez should have the final say.

I updated the first comment to include a capture that illustrates some of the changes here. Accepting, but I think @pablovazquez should have the final say.

Harley Acheson approved these changes 2024-03-22 17:30:16 +01:00

Harley Acheson left a comment

Thanks!

Pablo Vazquez approved these changes 2024-03-27 19:14:11 +01:00

Pablo Vazquez left a comment

Makes sense to unify this. Thanks!

Pratik Borhade commented

2024-03-28 04:49:17 +01:00

@blender-bot build

Milan Davidović commented

2024-03-28 10:08:02 +01:00

Just to check, I should merge main rather than rebase it and force push?

Pratik Borhade commented

2024-03-28 10:16:08 +01:00

@MilanDavidovic hi, can you merge latest main in this branch? So we can run buildbot again (just to play on safer side 🙂). Builds are failing due to old code.

- make update (make sure you're on main branch to pull latest main)
- git checkout fix-annotation-missing-widgets
- git merge main
- git push me fix-annotation-missing-widgets

These sequence of commands might help ^

@MilanDavidovic hi, can you merge latest main in this branch? So we can run buildbot again (just to play on safer side 🙂). Builds are failing due to old code. ``` - make update (make sure you're on main branch to pull latest main) - git checkout fix-annotation-missing-widgets - git merge main - git push me fix-annotation-missing-widgets ``` These sequence of commands might help ^

👍 1

Milan Davidović commented

2024-03-28 10:19:41 +01:00

Great, thanks!

Milan Davidović added 1 commit 2024-03-28 10:41:47 +01:00

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

350d4ea176 Merge branch 'main' into fix-annotation-missing-widgets

Pratik Borhade commented

2024-03-28 10:43:06 +01:00

@blender-bot build

Milan Davidović added 1 commit 2024-03-28 10:56:19 +01:00

4faacc5478 Fixed formatting.

Pratik Borhade reviewed 2024-03-28 11:25:12 +01:00

scripts/startup/bl_ui/properties_grease_pencil_common.py Outdated

						
				@ -418,3 +418,3 @@

				    def draw_header(self, context):

				        if context.space_data.type not in {'VIEW_3D', 'TOPBAR', 'SEQUENCE_EDITOR'}:

				        if context.space_data.type not in {'VIEW_3D', 'TOPBAR', 'SEQUENCE_EDITOR', 'IMAGE_EDITOR', 'NODE_EDITOR', 'PROPERTIES'}:

Pratik Borhade commented

2024-03-28 11:25:12 +01:00

Thanks, one more formatting change is detected https://builder.blender.org/admin/#/builders/210/builds/1270 :

-        if context.space_data.type not in {'VIEW_3D', 'TOPBAR', 'SEQUENCE_EDITOR', 'IMAGE_EDITOR', 'NODE_EDITOR', 'PROPERTIES'}:
+        if context.space_data.type not in {
+            'VIEW_3D',
+            'TOPBAR',
+            'SEQUENCE_EDITOR',
+            'IMAGE_EDITOR',
+            'NODE_EDITOR',
+                'PROPERTIES'}:
             self.layout.prop(context.space_data, "show_annotation", text="")

Thanks, one more formatting change is detected https://builder.blender.org/admin/#/builders/210/builds/1270 : ``` - if context.space_data.type not in {'VIEW_3D', 'TOPBAR', 'SEQUENCE_EDITOR', 'IMAGE_EDITOR', 'NODE_EDITOR', 'PROPERTIES'}: + if context.space_data.type not in { + 'VIEW_3D', + 'TOPBAR', + 'SEQUENCE_EDITOR', + 'IMAGE_EDITOR', + 'NODE_EDITOR', + 'PROPERTIES'}: self.layout.prop(context.space_data, "show_annotation", text="") ```

Milan Davidović commented

2024-03-28 11:31:19 +01:00

Thanks, make format didn't fix that one, I'll fix it now.

Thanks, `make format` didn't fix that one, I'll fix it now.

Pratik Borhade commented

2024-03-28 11:42:13 +01:00

This formatting change looks odd to me as well. But let's make the change for now :)