Fix #111212: Error running search if cursor is outside Blender window #111254

Christoph Lendenfeld · 2023-08-18T09:56:42+02:00

2023-08-18 09:56:42 +02:00

Fix the python poll function in NewGeometryNodeGroupTool so that it
does not error if the mouse is outside of the blender window.

Pressing F3 to call the search menu when the cursor is outside the Blender window causes CTX_wm_area(C) to return a nullptr which results in an error message in the console from the NewGeometryNodeGroupTool poll function in geometry_nodes.py. This PR updates that function to handle this situation by testing for lack of context.space_data.

Fix the python poll function in `NewGeometryNodeGroupTool` so that it does not error if the mouse is outside of the blender window. --- Pressing F3 to call the search menu when the cursor is outside the Blender window causes `CTX_wm_area(C)` to return a `nullptr` which results in an error message in the console from the `NewGeometryNodeGroupTool` poll function in `geometry_nodes.py`. This PR updates that function to handle this situation by testing for lack of `context.space_data`.

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2023-08-18 09:56:42 +02:00

Christoph Lendenfeld added 2 commits 2023-08-18 09:56:53 +02:00

045c16ba77 fix crash

c6f83ad86f don't always return false

Christoph Lendenfeld changed title from ~~Fix #111212: Crash when running search in non-active main window~~ to Fix #111212: Crash when running search while cursor is outside Blender window

2023-08-18 10:00:04 +02:00

Nathan Vegdahl requested review from Nathan Vegdahl 2023-08-18 10:41:15 +02:00

Harley Acheson added this to the User Interface project 2023-08-18 22:07:40 +02:00

Harley Acheson requested review from Harley Acheson 2023-08-18 22:07:50 +02:00

Harley Acheson commented

2023-08-18 22:14:48 +02:00

Hey @ChrisLend, thanks for working on this.

The comment doesn't look quite right as the minimum to reproduce this error is to have a single window, and active, but then press F3 while your mouse is outside of that window. So doesn't require multiple windows or to have anything inactive.

Your fix works to stop the exception, but there is still an error shown in the console from the NewGeometryNodeGroupTool python poll function:

  File "D:\blender-2.8-git\build_windows_Lite_x64_vc17_Release\bin\Debug\4.0\scripts\startup\bl_operators\geometry_nodes.py", line 269, in poll
    return context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL'
AttributeError: 'NoneType' object has no attribute 'type'

Note that C/C++ always does short-circuit evaluation, which can simplify the code a bit. A solution that seems to fix everything could look like this:

diff --git a/scripts/startup/bl_operators/geometry_nodes.py b/scripts/startup/bl_operators/geometry_nodes.py
index 776604cf397..33c9c52f30b 100644
--- a/scripts/startup/bl_operators/geometry_nodes.py
+++ b/scripts/startup/bl_operators/geometry_nodes.py
@@ -259,21 +259,21 @@ class NewGeometryNodeTreeAssign(Operator):
 
 
 class NewGeometryNodeGroupTool(Operator):
     """Create a new geometry node group for an tool"""
     bl_idname = "node.new_geometry_node_group_tool"
     bl_label = "New Geometry Node Tool Group"
     bl_options = {'REGISTER', 'UNDO'}
 
     @classmethod
     def poll(cls, context):
-        return context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL'
+        return context.space_data and context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL'
 
     def execute(self, context):
         group = geometry_node_group_empty_new()
         group.asset_mark()
         group.is_tool = True
         context.space_data.node_tree = group
         return {'FINISHED'}
 
 
 class SimulationZoneOperator:
diff --git a/source/blender/editors/screen/screen_ops.cc b/source/blender/editors/screen/screen_ops.cc
index dffce5bc921..fff8a21509d 100644
--- a/source/blender/editors/screen/screen_ops.cc
+++ b/source/blender/editors/screen/screen_ops.cc
@@ -3232,21 +3232,22 @@ static int keyframe_jump_exec(bContext *C, wmOperator *op)
   DEG_id_tag_update(&scene->id, ID_RECALC_FRAME_CHANGE);
 
   WM_event_add_notifier(C, NC_SCENE | ND_FRAME, scene);
 
   return OPERATOR_FINISHED;
 }
 
 static bool keyframe_jump_poll(bContext *C)
 {
   /* There is a keyframe jump operator specifically for the Graph Editor. */
-  return ED_operator_screenactive_norender(C) && CTX_wm_area(C)->spacetype != SPACE_GRAPH;
+  ScrArea *area = CTX_wm_area(C);
+  return ED_operator_screenactive_norender(C) && (!area || area->spacetype != SPACE_GRAPH);
 }
 
 static void SCREEN_OT_keyframe_jump(wmOperatorType *ot)
 {
   ot->name = "Jump to Keyframe";
   ot->description = "Jump to previous/next keyframe";
   ot->idname = "SCREEN_OT_keyframe_jump";
 
   ot->exec = keyframe_jump_exec;

Hey @ChrisLend, thanks for working on this. The comment doesn't look quite right as the minimum to reproduce this error is to have a single window, and active, but then press F3 while your mouse is outside of that window. So doesn't require multiple windows or to have anything inactive. Your fix works to stop the exception, but there is still an error shown in the console from the NewGeometryNodeGroupTool python poll function: ``` File "D:\blender-2.8-git\build_windows_Lite_x64_vc17_Release\bin\Debug\4.0\scripts\startup\bl_operators\geometry_nodes.py", line 269, in poll return context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL' AttributeError: 'NoneType' object has no attribute 'type' ``` Note that C/C++ always does short-circuit evaluation, which can simplify the code a bit. A solution that seems to fix everything could look like this: ```Diff diff --git a/scripts/startup/bl_operators/geometry_nodes.py b/scripts/startup/bl_operators/geometry_nodes.py index 776604cf397..33c9c52f30b 100644 --- a/scripts/startup/bl_operators/geometry_nodes.py +++ b/scripts/startup/bl_operators/geometry_nodes.py @@ -259,21 +259,21 @@ class NewGeometryNodeTreeAssign(Operator): class NewGeometryNodeGroupTool(Operator): """Create a new geometry node group for an tool""" bl_idname = "node.new_geometry_node_group_tool" bl_label = "New Geometry Node Tool Group" bl_options = {'REGISTER', 'UNDO'} @classmethod def poll(cls, context): - return context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL' + return context.space_data and context.space_data.type == 'NODE_EDITOR' and context.space_data.geometry_nodes_type == 'TOOL' def execute(self, context): group = geometry_node_group_empty_new() group.asset_mark() group.is_tool = True context.space_data.node_tree = group return {'FINISHED'} class SimulationZoneOperator: diff --git a/source/blender/editors/screen/screen_ops.cc b/source/blender/editors/screen/screen_ops.cc index dffce5bc921..fff8a21509d 100644 --- a/source/blender/editors/screen/screen_ops.cc +++ b/source/blender/editors/screen/screen_ops.cc @@ -3232,21 +3232,22 @@ static int keyframe_jump_exec(bContext *C, wmOperator *op) DEG_id_tag_update(&scene->id, ID_RECALC_FRAME_CHANGE); WM_event_add_notifier(C, NC_SCENE | ND_FRAME, scene); return OPERATOR_FINISHED; } static bool keyframe_jump_poll(bContext *C) { /* There is a keyframe jump operator specifically for the Graph Editor. */ - return ED_operator_screenactive_norender(C) && CTX_wm_area(C)->spacetype != SPACE_GRAPH; + ScrArea *area = CTX_wm_area(C); + return ED_operator_screenactive_norender(C) && (!area || area->spacetype != SPACE_GRAPH); } static void SCREEN_OT_keyframe_jump(wmOperatorType *ot) { ot->name = "Jump to Keyframe"; ot->description = "Jump to previous/next keyframe"; ot->idname = "SCREEN_OT_keyframe_jump"; ot->exec = keyframe_jump_exec; ```

Christoph Lendenfeld added 2 commits 2023-08-24 12:22:15 +02:00

ca7e5c7b99 Merge branch 'main' into fix_main_window_search_crash

f64a710839 implement review suggestions

Christoph Lendenfeld added 1 commit 2023-08-24 12:29:01 +02:00

2666a7d59a got the bool logic wrong

Christoph Lendenfeld commented

2023-08-24 12:30:50 +02:00

@Harley fixed the Geometry node tool as well

Harley Acheson approved these changes 2023-08-24 17:50:51 +02:00

Harley Acheson left a comment

Works Great!

Julian Eisel commented

2023-08-25 19:59:56 +02:00

Yikes! Only saw this after committing 56781c80b7 and scrolling over the report again - too easy to miss PR attachments on Gitea... My fix does the same in effect, just a bit more elegant I'd say. Feel free to commit the node tool fix included here, that should be committed separately anyway (and looks fine).

Yikes! Only saw this after committing 56781c80b7 and scrolling over the report again - too easy to miss PR attachments on Gitea... My fix does the same in effect, just a bit more elegant I'd say. Feel free to commit the node tool fix included here, that should be committed separately anyway (and looks fine).

Harley Acheson commented

2023-08-25 20:11:11 +02:00

@JulianEisel - My fix does the same in effect, just a bit more elegant I'd say.

Yours looks like it would fix the reported error, in that it will work (and return false) if the ScrArea is null, as we get when the mouse is outside the window. However, it appears to reverse the current test there, changing from !SPACE_GRAPH to SPACE_GRAPH. From the comment above it looks like it meant to return false if in graph editor so that a more specific operator can do this instead.

> @JulianEisel - My fix does the same in effect, just a bit more elegant I'd say. Yours looks like it would fix the reported error, in that it will work (and return false) if the ScrArea is null, as we get when the mouse is outside the window. However, it appears to reverse the current test there, changing from !SPACE_GRAPH to SPACE_GRAPH. From the comment above it looks like it meant to return false if in graph editor so that a more specific operator can do this instead.

Julian Eisel commented

2023-08-25 20:25:58 +02:00

Yours looks like it would fix the reported error, in that it will work (and return false) if the ScrArea is null, as we get when the mouse is outside the window. However, it appears to reverse the current test there, changing from !SPACE_GRAPH to SPACE_GRAPH. From the comment above it looks like it meant to return false if in graph editor so that a more specific operator can do this instead.

Good catch, thanks! Clearly weekend time o/ (Committed 211d631428)

> Yours looks like it would fix the reported error, in that it will work (and return false) if the ScrArea is null, as we get when the mouse is outside the window. However, it appears to reverse the current test there, changing from !SPACE_GRAPH to SPACE_GRAPH. From the comment above it looks like it meant to return false if in graph editor so that a more specific operator can do this instead. Good catch, thanks! Clearly weekend time o/ (Committed 211d631428)

Harley Acheson commented

2023-08-25 21:34:30 +02:00

@ChrisLend

As noted above, main now contains a working fix for screen_ops.cc in place. So we're hoping that you can update this PR to include only the changes to geometry_nodes.py and then merge without need for further approval - Julian said "looks fine".

Thanks!

@ChrisLend As noted above, `main` now contains a working fix for `screen_ops.cc` in place. So we're hoping that you can update this PR to include only the changes to `geometry_nodes.py` and then merge without need for further approval - Julian said "looks fine". Thanks!

Harley Acheson changed title from ~~Fix #111212: Crash when running search while cursor is outside Blender window~~ to Fix #111212: Error when running search while cursor is outside Blender window

2023-08-25 21:40:50 +02:00

Harley Acheson changed title from ~~Fix #111212: Error when running search while cursor is outside Blender window~~ to Fix #111212: Error running search if cursor is outside Blender window

2023-08-25 21:41:36 +02:00

Harley Acheson commented

2023-08-29 19:48:45 +02:00

@ChrisLend

The issue that this PR originally was addressing was fixed in 56781c80b7. The remaining change is a bit too unrelated to the original issue, and I don't want to just retitle since we'll still have a funny history.

Instead I will make a new PR for the change to geometry_nodes.py and merge with you as author. Then I'll close this one.

@ChrisLend The issue that this PR originally was addressing was fixed in 56781c80b7. The remaining change is a bit too unrelated to the original issue, and I don't want to just retitle since we'll still have a funny history. Instead I will make a new PR for the change to geometry_nodes.py and merge with **you as author**. Then I'll close this one.

Harley Acheson commented

2023-08-29 20:38:12 +02:00

Committed the remaining part of this solution in 468584f5e7
Closing this as mentioned earlier.

Committed the remaining part of this solution in 468584f5e7 Closing this as mentioned earlier.

Harley Acheson commented

2023-08-29 20:38:14 +02:00

Committed the remaining part of this solution in 468584f5e7
Closing this as mentioned earlier.

Committed the remaining part of this solution in 468584f5e7 Closing this as mentioned earlier.

Harley Acheson closed this pull request

2023-08-29 20:38:14 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #111212: Error running search if cursor is outside Blender window #111254

Pull request closed