blender/blender

Fix #98014: Preserve tool when exiting full screen area #119856

Closed

YimingWu wants to merge 2 commits from ChengduLittleA/blender:fix-98014 into main

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

YimingWu commented

2024-03-25 03:50:11 +01:00

Member

Since some tools share same brush fields in toolsettings, the tool can
get overwritten when exiting fullscreen and multiple ScrAreas having
different brush settings (like a image editor space could have different
texture paint tool than what is selected in the 3d viewport in texture
paint mode).

This patch fixes the problem by putting the originalScrArea to the
last in screen->areabase, so when exiting fullscreen, the tool gets
preserved since it's always the last one to be set.

Since some tools share same brush fields in `toolsettings`, the tool can get overwritten when exiting fullscreen and multiple `ScrArea`s having different brush settings (like a image editor space could have different texture paint tool than what is selected in the 3d viewport in texture paint mode). This patch fixes the problem by putting the original`ScrArea` to the last in `screen->areabase`, so when exiting fullscreen, the tool gets preserved since it's always the last one to be set.

YimingWu added the

Sculpt, Paint & Texture

labels 2024-03-25 03:50:12 +01:00

YimingWu added 1 commit 2024-03-25 03:50:21 +01:00

Fix #98014 : Preserve tool when exiting full screen area c0a661d3ab

Since some tools share same brush fields in `toolsettings`, the tool can
get overwritten when exiting fullscreen and multiple `ScrArea`s having
different brush settings (like a image editor space could have different
texture paint tool than what is selected in the 3d viewport in texture
paint mode).

This patch fixes the problem by putting the original`ScrArea` to the
last in `screen->areabase`, so when exiting fullscreen, the tool gets
preserved since it's always the last one to be set.

YimingWu requested review from Pablo Vazquez 2024-03-25 03:50:54 +01:00

YimingWu requested review from Simon Thommes 2024-03-25 03:50:54 +01:00

Simon Thommes approved these changes 2024-03-25 11:42:05 +01:00

Simon Thommes left a comment

Member

Thanks for the fix, works for me!

Would probably be good though to have someone else at least quickly look over it, since I can't do the code review and wouldn't know if there's another preferred way of doing this.

Thanks for the fix, works for me! Would probably be good though to have someone else at least quickly look over it, since I can't do the code review and wouldn't know if there's another preferred way of doing this.

YimingWu requested review from Sergey Sharybin 2024-03-25 12:09:45 +01:00

Sergey Sharybin commented

2024-03-25 12:29:13 +01:00

Owner

This isn't really my area.

@JulianEisel is it something you can review?

This isn't really my area. @JulianEisel is it something you can review?

Pratik Borhade requested review from Julian Eisel 2024-04-15 09:19:39 +02:00

Pablo Vazquez commented

2024-05-14 19:47:46 +02:00

Member

@JulianEisel The fix is only a couple of lines, do you think it is the right approach or some other way should be investigated?

@JulianEisel The fix is only a couple of lines, do you think it is the right approach or some other way should be investigated?

YimingWu added 1 commit 2024-08-28 07:48:07 +02:00

Merge branch 'main' into fix-98014 d2445df2ae

Julian Eisel commented

2024-08-28 19:01:29 +02:00

Member

Relying on the order of areas in the screen for a tool system feature is quite weak, and can break easily. Let's not create such hidden logical dependencies. I rather have some explicit heuristics select an active tool as the screen state changes.

Following discussion in !98014 this seems solved for now, so I guess we can close this anyway?

Relying on the order of areas in the screen for a tool system feature is quite weak, and can break easily. Let's not create such hidden logical dependencies. I rather have some explicit heuristics select an active tool as the screen state changes. Following discussion in !98014 this seems solved for now, so I guess we can close this anyway?

Julian Eisel requested changes 2024-08-28 19:01:50 +02:00

Julian Eisel left a comment

Member

Requesting changes, see previous comment.

Requesting changes, see previous comment.

YimingWu commented

2024-08-29 05:36:38 +02:00

Author

Member

Yeah seems that we could close this because the brush asset system has apparently solved the issue.

Yeah seems that we could close this because the brush asset system has apparently solved the issue.

YimingWu closed this pull request

2024-08-29 05:36:41 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Automated Testing

Blender Asset Bundle

Code Documentation

Dependency Graph

Development Management

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Viewport & EEVEE

Virtual Reality

Asset Browser Project

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Viewport & EEVEE

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

5 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#119856

No description provided.