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

Closed
Christoph Lendenfeld wants to merge 5 commits from ChrisLend:fix_main_window_search_crash into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

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
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
Member

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
Christoph Lendenfeld added 1 commit 2023-08-24 12:29:01 +02:00
Author
Member

@Harley fixed the Geometry node tool as well

@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
Member

Works Great!

Works Great!
Member

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).
Member

@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.
Member

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)
Member

@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
Member

@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.
Member

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.
Member

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

Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 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#111254
No description provided.