-
Notifications
You must be signed in to change notification settings - Fork 150
Fix/imagerescale events #1127
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?
Fix/imagerescale events #1127
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial thoughts. Can you explain I more detail how this is a problem? why do we even need to pass on the imageSize? if we do the translations correctly according to this, should we not end up with the original image size in that case? then the consumer could just use that size and the normalized coords or?
Given that it is needed, should the eventscaler not be in base or core?
Also does it handle cases where there aspect does not match? then one would need to adjust the positions to or? Should this functionality not go in the imageinport propagate event?
Is there a common base for events that hold a canvas size? If not why not add one? instead of repeating the various events... |
It is only a problem if one tries to calculate the integer pixel position of an event and expect it to match the ImageOutport of a processor. Assuming that one has an image scaling processor that downsizes by 2 and then use an event that has propagated through that processor you could end up outside of the image outport in your processor “above” the scaling. |
In case your processor does not have an image outport but need the event and wants to know the integer pixel position |
Hmm, probably. In case the “original” canvas size is needed somewhere we could add that to the event |
we could move it to InteractionEvent. I considered it but never did it. |
When would you need the pixel position of the event and not have the image in question at hand? |
Key events don't have a size I think. and those derive InteractionEvent, MouseInteractionEvent has the size, but TouchEvents inherits Interaction Event and also have a size. So Now would need an extra class between InteractionEvent and MouseInteractionEvent that also TOuchEvent can derive from |
For convenience in many cases I guess. One could probably forward the size in most cases |
Did a bit of research. Moving this work into the ImageInportWe could override Inport::propagateEvent(Event* event, Outport* target) - create a copy of each event and set the canvas size to match that of the ImageInport. I'm not sure which solution is best here. |
Agree I would probably be nice to have the size in the event. Did you think anything more on the aspect ratio mismatch, Im thinking about the cases when we add black padding to an image for example. Also does this interact with the image port resize behavior, OutpotDetermainsSize and so on? |
The ratio-matching cases will probably need to do something similar to the ViewManager. A bit more complicated than the problem I had in mind...
It is based on the actual size of the inport so I think that should not be a problem. I tried a couple of different settings in the ImageResampler manually and it seemed to work as intended. One would have to take care that the EventScaler has the correct size set. We could use a lambda here to avoid duplicating state. |
…ort dimensions does not match BaseGL: ImageResample fix for empty inport on resize Jenkins: Format fixes
dd66037
to
8cad598
Compare
Ensure that canvasSize of events matches the size of rescaled images. This behaviour matches that of the ViewManager