Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Vector4/i properties show in 2x2 layout #80893

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Aug 22, 2023

Vector4 version of #45713

Before:
image

After:
image

With link:
image

This is going to be especially relevant for Vector4i properties after #79021
It also allows to unify Rect and Vector editor properties.

@@ -3708,14 +3708,14 @@ EditorProperty *EditorInspectorDefaultPlugin::get_editor_for_property(Object *p_

} break;
case Variant::VECTOR4: {
EditorPropertyVector4 *editor = memnew(EditorPropertyVector4);
EditorPropertyVector4 *editor = memnew(EditorPropertyVector4(p_wide));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change. I just noticed it's missing.

@KoBeWi KoBeWi requested a review from a team August 22, 2023 13:41
@MewPurPur
Copy link
Contributor

What about Plane?

I think the other one went through more easily because xy and hw are separate concepts, but here xyzw are just positions in identical dimensions

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 22, 2023

What about Plane?

Looks like another duplicate property 🙄
I can unify it together with Rect properties and it will get 2x2 layout for free.

I think the other one went through more easily because xy and hw are separate concepts, but here xyzw are just positions in identical dimensions

The original PR was about having more space, I never considered the concepts. Vector4 didn't exist then and I don't use Plane, so I didn't know it also has this problem (though Rect2 is probably used more in general).

@fire
Copy link
Member

fire commented Aug 22, 2023

A vector4 doesn't naturally have a 2x2 structure like a transform 2d has. Can you explain this change?

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 22, 2023

See godotengine/godot-proposals#2237
Same problem. The horizontal layout is too small for 4 fields.

@bitsawer
Copy link
Member

I'm not totally sure about this. Especially when I work with shaders a lot with many vec3, vec4 and mat4 types I feel this can make the UI a bit more confusing. Keeping all vector values in one row (except for weirdo Vector2) makes eyeballing the UI easy and there is a visual difference between vectors and matrices. Making Vector4 take two rows makes it easy to mistake for a matrix.

And if Vector4 takes too much space horizontally, what should mat4 or Projection look like? It also takes four slots horizontally right now and it's hard to image how the new layout would look like without breaking the currently 1-to-1 visual to data mapping.

shader

@Mickeon
Copy link
Contributor

Mickeon commented Sep 2, 2024

I think this should be tied to another editor setting like interface/inspector/horizontal_vector2_editing and interface/inspector/horizontal_vector_types_editing, although it's really pushing it. The latter is also a "lie" if this PR displays it this way regardless of the setting.

@fire
Copy link
Member

fire commented Sep 3, 2024

I think it would be confusing to add 2x2 layout for Vector4, but I'm not strongly attached to that position.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants