-
Notifications
You must be signed in to change notification settings - Fork 26
Initial concept for scaling imported images to the scene's field chart #66
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
base: master
Are you sure you want to change the base?
Initial concept for scaling imported images to the scene's field chart #66
Conversation
Added new param to end of func for compatibility, but maybe it could be a new value for 'alignment' `PROJECT_RESOLUTION` which would be `ASIS` + the scaling logic? I'm not sure if my logic for scaling the image is short-sighted in some way, since I haven't worked with scenes with non-default field setups. Since oH historically would have defaulted to 12 fields, maybe this logic should as well, in case anyone was relying on that behaviour. Another concept would be to have an extra param that could be any of `FIT_Y`, `FIT_X`, `PROJECT_RESOLUTION` or a scale factor for manual adjustment. The benefit I see for this method of setting fields is that the user can set the scale to 1 and the image will go back to the 'correct' size. The drawback is that it's different to how the GUI import process works, so images brought in via script and via user would mysteriously not work with the same pegs
|
that's a very interesting idea, thanks! I think there might be an issue because I don't see a modification of the addElement function to account for the new parameter? Another thing that might require a little testing is whether the function runs fine in -batch (headless) mode? As I think the CELIO class has less capabilities in that mode. In that case we should avoid potential crashes due to the missing function. Have you tested headless? |
|
I haven’t tested headless, I’ll do that next, good plan! The good news is that this is the only modification needed to make it work; the addElement func already accepts a fields param, with a default value of 12 |
A scene with a field grid described as "12 Fields" actually has 24x24 units. The `element.add()` function takes a 'field guide' arg, which corresponds not to the number of field units, but to an integer for the 12 in 12-field (which means 24). So all my imports were off by a factor of 2. I have tested float values and it behaves as expected, so it *should* handle weird field guides if people set up od numbers or something
|
I have tested this in batch and it works as expected. A lingering issue is how it should interact with the TVG scaling arg. Currently that arg does nothing for non tvgs, but maybe we could clear that up and make it more cohesive? With openHarmonyAn image that's converted to TVG will by default fit into the Y height of the scene by reducing the node's scale attribute. If you set scale = 1, the TVG will be at actual pixels. A non-tvg will fit into the Y height of the scene by adjusting what scale = 1 means for the element. Setting scale = 1 will make the image fit the scene's Y height, and you need to scale a larger image up to make it 'actual size' (or a smaller image down) With the UIIf you import an image and convert it to a TVG, you can choose "actual size" it brings it in at scale = 1 at its actual size If you import an image and don't convert to TVG, you can choose "project resolution" as the size setting. This imports the image the same as oH where scale = 1 fits it to Y height, but it sets the scale attribute of the image so it's the right size, scaling a larger than project resolution image up, or a smaller one down |
|
hmm it's still a bit obscure to me with that description haha, but I trust you! Are you satisfied with the current capability of the PR? |
|
I've met our immediate production needs w the PR as-is, but I'm not satisfied with it yet as a neat solution. I think what I would like to do is use the resize_axis argument and have it affect tvgs and non tvgs in the same way. But I'm worried this could result in scripts people have written that compensate for the weird behavior working differently, and we want to avoid breaking changes |
Added new param to end of func for compatibility, but maybe it could be a new value for 'alignment':
PROJECT_RESOLUTIONwhich would beASIS+ the scaling logic?I'm not sure if my logic for scaling the image is short-sighted in some way, since I haven't worked with scenes with non-default field setups. Since oH historically would have defaulted to 12 fields for new elements, maybe this logic should as well, in case anyone was relying on that behaviour.
Another concept would be to have an extra param that could be any of
FIT_Y,FIT_X,PROJECT_RESOLUTIONor a scale factor for manual adjustment.The benefit I see for this method of setting fields is that the user can set the scale to 1 and the image will go back to the 'correct' size. The drawback is that it's different to how the GUI import process works, so images brought in via script and via user would mysteriously not work with the same pegs.
I could add this to the other importing funcs once we're more solid on how it makes sense to work.