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

Polygon / Transform Editing Updates Enhancement and Fixes #358

Open
InnerScript opened this issue Jan 26, 2016 · 12 comments
Open

Polygon / Transform Editing Updates Enhancement and Fixes #358

InnerScript opened this issue Jan 26, 2016 · 12 comments

Comments

@InnerScript
Copy link

I wanted to file this as a separate issue for discussion, because it's touching a number of components, including the libGDX runtime.

Currently my focus is addressing the behavior of polygons, which has an overlap with Issue #351 and #165. The scope is getting larger the more I dig into the code, so decided to make a separate issue.

My thoughts are to make editing polygons more closely match the basic behavior in other vector/polygon editors such as Illustrator, Flash, Maya, etc.

Issues to fix:

  1. Make the bounding box correctly follow the polygon when shape editing (almost complete)
  2. Make the transform tool correctly follow the polygon when shape editing (almost complete)
  3. Add an editable local pivot point/vertex origin
  4. Fix instances of polygons sourced from the library
  5. Address the logical issue of scaling/dimensions with source data that is variable
  6. Shore up any export issues

Number 5 is arguably the most involved, and may result in a breaking change.

The issue comes to a head when working on point data directly, or any source data where the internal dimension may change. For example, you have a 20x20 octagon polygon at 1.0 scale. When you drag the left most point 10 pixels left, the implicit size is now 30x20. When you set the width to 40, now the vertexes must be stretched accordingly to 40x20. But, what if the scale was then set to 2.0? The actual width is now actually 80, but in the object is set to 40. What was of more value for the width, 80 or 40?

My thoughts are for non-library polygons, to remove scaleX/scaleY as values to be set. The width/height and transform tool dimensions will function to reposition the vertices.

In fact, I'll argue that even though libGDX implements scaleX/scaleY and width/height on the same element, that we should restrict their use in the entire editor. The restriction would be that for any element only scale OR size should be editable, but not both at the same time. One then becomes implicit to the other. Or even better, remove scaleX/scaleY completely as editable values, as it is what introduces the issue, no other graphic tool behaves similarly, and there is little value added for a visual editor.

Consider another example of this conundrum. When a source image size changes, what does the user want? For all instances to be resized, or for all to remain in the same position and size but with updated detail? We can't determine that, but having both scale and size in place makes little sense as the behavior will be unpredictable when the source data changes. Having one or the other as the principal means to resize elements gives the user the option to chose the behavior when source data changes.

@azakhary
Copy link
Contributor

Thanks for bringing this up, this is an important topic. I'll get back to this in more detail today.
But just as few key points:

  1. The part with bounding box is already being fixed by someone, I've seen PR yesterday, and even probably merged. I'll check that.
  2. The width/scale issue is real and I completely agree, and breaks so many things. I want to discuss more on this. and actually deciding on this is important to go further.
  3. Origin point editing. It's actually there in sources, just commented out. We can add this part to the follower of polygons, while keeping out for other items. Why keep out on other items? because there are bugs related to transforms when origin is being changed. It's pretty hard to implement. Lot's of math.

@InnerScript
Copy link
Author

  1. Understood, I saw some existing PR's that were related, but there's also code I added specifically to polygonal editing that won't interfere. (Since the user can interactively change the polygon's dimensions interactively by dragging points around) I'll keep an eye out for an update.
  2. I've thought quite a bit about this, and have experience in the matter...
  3. ... so the math doesn't scare me much. :) I saw where some origin handle code was commented out, and I'm working to make it functional. I won't check in a PR for unless it works, at least for polygons.

@azakhary
Copy link
Contributor

Well, if you can pull this off this would be really amazing.
I also think and agree that we should ditch scale in favor of width/height. But for some cases like sprites/animations the "original" width/height of texture should be displayed somewhere (maybe, texture properties panel is a good idea) this will solve tons of problems.

If there is anything I can help with, let me know. Or if there is any topic you want me to dive in in more detail.

@InnerScript
Copy link
Author

Thanks for the support.

I agree that the original dimensions would be good to display, but for now will leave that for a separate enhancement issue after this is complete. Likely along with the original width/height, for bitmaps the most useful display would be a ratio of the current pixel density, taking into account the original dimensions and the current target resolution.

@kschoos
Copy link

kschoos commented Jan 27, 2016

o/
I feel like this should be planned thoroughly to avoid having to touch transformations ever again.
It feels broken and fragmented in the current state and, before making changes I would suggest creating a plan of action for this involving:
A Class diagram of the current state
A well thought-out model of how the whole transformation should fit together, considering everything.

Really, lets make this good. Please. Why are there only bounding boxes for Composite Items? Why, when I pull the lower left handle, the object transforms to the upper right?

What do you think?

@azakhary
Copy link
Contributor

Agreed that this is important. There are several things to consider:

  1. There should be some box that wraps around the item visually, for follower. and reflects items size. We call this currently "Dimension", it can be edited with transform tool. If you change dimension of sprite, it visually stretches. If you say change dimension of composite, nothing should happen, unless we later have things that align to walls. I am yet unsure if there can be possibly any reason to NOT start the dimension rect from 0,0 of item local coordinates.
  2. There should be another thing, a kind of "bound" thingie that has to be a polygon or rect or other shape (circle), for collision detection or physics. This is a separate thing. it has nothing to do with dimension. Dimension can be only rect. This one can be any shape.

This particular issue, is about making Transform tool working properly with this 2 things, and also un-commenting the "origin" point modifying functionality. resizing/repositioning/rotating transformation should be applicable to both "bound box" , and also "dimension". And for that they both need to have origin points (so we can rotate them)

Does this make sense?

@kschoos
Copy link

kschoos commented Jan 27, 2016

Couldn't Composite and CustomSizeObject be two different things?

So that CompositeItem is only a container with a Position and Scale Transform as well as an AABB (We are clear about what AABB is, right? It's the smallest possible Rectangle that snuggles around all the objects inside and that is axis aligned, so that it always has 2 sides parallel to the x and 2 sides parallel to the y axis.)
And there is BoxContainer or ViewportRect or such. You can draw it from the Toolshelf, it's just like adding an image or picture, but it does not render anything. If you then had
CompositeItem
|> BoxContainer
|> ObjectA
|> ObjectB

Setting the scale on CompositeItem would alter the scale of all of the objects, keeping their relative positions to each other adequate.
Resizing the BoxContainer itself would resize the BoxContainer without resizing it's inner objects.
There would just need to be a way to link an object to a specific BoxContainer to allow for easy use of "align" features.

@azakhary
Copy link
Contributor

We can have an empty object that has size. But that does not mean we cannot also make our Composite be such an object as well. In fact we already have this kind of "components" this are dimension component and polygon component. They are the ones to mean the 2 things I described earlier.

@InnerScript
Copy link
Author

What is the value of having an object size be different than it's graphical/world dimensions? Is there a use case for this that I'm unaware of?

Again, keeping in line with the way other professional graphic editors work, they do not have this concept.

@azakhary
Copy link
Contributor

If you are talking about - Bounding Box for collisions and physics. It has nothing to do with graphical representation. Even in Unity, you can see them modify the box unrelated to graphical representation, so that the collision happens few pixels inside the image.

If you are talking about - Dimension (not the bounding box for collisions/physics) then you are correct.
And I did not suggest it be different from graphical dimensions. It should reflect the graphical view exactly.
If it's an image and you change the dimension, image should visually stretch. The only use case when it's different is when it's composite. Composite is a box of items. You can make composite bigger or smaller, but items will remain same size. This can be needed when you do UI. for example imagine you have composite-progress bar. Inside it 2 nine patch items, one for BG, one for progress bar, also a label that with number. Later on we can have a feature, where composite children can have relative positions (example: align center to parent, width=100% of parent e.g) so that when we change composite size, it will again reflect things visually on all children. But only with logic user specified.
There is almost no normal use case when person wants to stretch and distort composite by changing it's size (it's also suboptimal causing additional GPU call, which means a person should avoid this) Although this stretching can be left as well, withs some additional checkbox.

I hope I explained things well. But if we are all still not fully understanding each other, maybe it's worth doing a voice group chat for this reason? Let me know.

@InnerScript
Copy link
Author

@azakhary

Thanks for taking the time to explain, it was explained clearly.

Reading what you described, my first inclination is to suggest that things that exist in the world should be different classes than things that exist in the UI. On top of needing the slightly different behavior, the UI is usually in screen space, not world space. (rendered with a separate projection)

How does a composite logically differ from a group (as other programs implement it)?

Did you intend composite items to exist in both world and screen space?

@azakhary
Copy link
Contributor

azakhary commented Feb 7, 2016

Agreed regarding the UI, although this part can bed one later.

I do not thing composite logically differs from a group. It's exact same thing.

I am not sure about the last question. composite items just exist, they are just Node groups that group items together, they exist everywhere you "put" them. They coordinates are currently in world space, but thanks to pixel per world they can be easily adjusted to anywhere.

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

No branches or pull requests

3 participants