Fix multiple usability issues in the texture region editor #80435
Conversation
YuriSizov
force-pushed
the
region-editor-quality-pass
branch
from
343d0bd to
987c715
Compare
YuriSizov
force-pushed
the
region-editor-quality-pass
branch
from
987c715 to
fb57fda
Compare
There was a problem hiding this comment.
As @akien-mga said, this seems fine. Didn't review the code extensively, but it looks good.
- Correctly display atlas textures when used by other objects. - Make region handles easier to hit in ninepatchable objects. - Correctly initialize and restore various visual properties. - Improve code quality.
YuriSizov
force-pushed
the
region-editor-quality-pass
branch
from
fb57fda to
4b7d0c8
Compare
There was a problem hiding this comment.
I've done some testing, LGTM!
Tried multi-nested AtlasTextures with non-zero margins. Even for these both texture rendering and region selecting is working correctly.
I've also played a little with StyleBoxTexture or NinePatchRect, haven't had any issues.
Note there are still cases where the texture (for which the region is being edited) will be rendered "incorrectly". E.g. when selecting a region for a CanvasTexture with an AtlasTexture assigned to its diffuse texture, such texture would't be rendered "correctly" within the region editor. But such specific combination doesn't work properly even outside of the region editor (I think using AtlasTextures within CanvasTexture is not supported), meaning that's not an issue with the region editor itself.
AFAICT, what's supported seems to work just fine. 👍
(haven't tested everything though; but for sure things work better than before 🙃)
There's this small issue when editing a (0, 0, 0, 0) region:
After opening the region editor it shows as if the whole texture was selected as the region:
Closing the region editor without changes makes the region remain being (0, 0, 0, 0). This might be surprising given within the texture editor the whole texture was shown to be selected.
Any slight region-drag within the region editor is needed to make the region be set accordingly within the Sprite2D etc. (even when snapping so no effective change):
Not sure if anything/what needs to be done about this.
I can revert some changes, because that's on me. I unified some of that logic that used to treat these as special cases (or rather other empty regions as special cases). I think, though, that it's more intuitive for an empty region to equal the whole image. At least it is more user friendly when opening the region editor, in my mind. But I guess there is a different between an empty region and and empty region with a "use region" toggle. So I dunno. We can keep it as is and see if there are complaints after the change, I guess. |
|
Thanks! |
|
Help. please" |
|
@ramireshimself You cannot dock it, no. It used to be a contextual editor on the bottom panel, but this was changed in Godot 4. |
|
Thanks for the awenser. So there is no simmilar way to do like before? I have to do it one by one, closing, keyfaming and opening again, and edit, etc, etc? |
|
You can input the value in the inspector and only use the dialog to set the first frame. |
|
thnks, I´ll try. |
Fixes #75291. Fixes #80403 (a duplicate, actually). Fixes #75989 (at least, I can't reproduce it with the PR applied).
Changes include:
Correctly display atlas textures when they are used by other supported objects (like nodes or styleboxes). We used to draw everything through a
CanvasTexture, but it can't handle custom drawing logic of textures that it holds, such asAtlasTexture. So I reworked it to avoid relying on theCanvasTexture.Make region handles easier to interact with when editing NinePatchRect and StyleBoxTexture, which also have bars to adjust margins. We used to check handles last, which means bars would steal clicks. Since bars are normally long and can be clicked in plenty of places, and handles are small and located in precise positions, I switched it so handles are checked first.
While testing I've noticed that a few things weren't properly initialized or restored. To name a couple, the default snap mode from the editor settings wasn't correctly applied, and the panning offset wouldn't completely update when opening the dialog.
Various code quality changes. Moved all property initializations into the header. Removed unused methods, changed access modifiers of some remaining. Renamed everything. Extracted a few bits of logic into dedicated methods and unified how those are used. And other such changes.
Add a check to
AtlasTexture::get_image()to avoid bliting with invalid size values. It spammed a lot in the editor when using the region editor otherwise.Apparently, you can use Godot's nose to create a nice ninepatch: