GNOME Bugzilla – Bug 744763
gltransformation: Make all property into universal [0-1] coordinate
Last modified: 2015-08-16 13:39:45 UTC
Currently the translation properties range to is base on the video proportion. It's a bit a work for the application to figure this out and do that math, specially that aspect ratio may change if the video change. What I'm proposing instead, is to change the properties, so they are expected in universal coordinate, like in GL. Where a translation of +1 would make the image go out of view.
Created attachment 297237 [details] [review] gltransformation: Fix doc typo
Created attachment 297238 [details] [review] gltransformation: normalize translations
The gltransformation plugin was designed with a visual editor in mind. In order to normalize the coordinates to 1 for every aspect ratio, the translation vector would need to be modified when moving the image with a mouse cursor, i.e. the X pixels will need to be multiplied with the inverse apect ratio, so they won't be as fine as with the previous solution. Also the image is scaled and rotated after being translated. This is implemented this way to achieve an intuitive scale / rotation operation where the image is located. Otherwise the image will move while scaled. The range of your patch needs to be increased, since a scaled image can't be moved out of the canvas this way. The translation property would be out of range: Warning: value "2.709814" of type 'gfloat' is invalid or out of range for property 'translation-x' of type 'gfloat' With this in mind the normalized coordinates only make sense for unscaled images and the question is if this is usefull then. For a better understanding of the transformation values, you could try my gui for the plugin: https://github.com/lubosz/gst-gl-tests You need gst-plugins-bad on 1200f2f94bfaaffb8e56b9e7bb8eca34c7a765fc, since it does not work on master currently because of a regression. Also you need python-cairo from git. The "pivot" point of the transformation is currently hardcoded to the center of the image. I was going to make this an option. So the scale and rotation operations could be made with another origin. Thanks for finding the typo and your interest in the plugin :)
Ok, so it really depend on the use case. Maybe it could be configurable with properties: - "pivot" property could be an enum {CENTER, ORIGIN} - "translation-normalized" boolean where FALSE means how it is now, and TRUE means that translation happens after scale and implies pivot=origin, so translation-x would be always in [-1,1] range?
tbh, the part I care the most is normalize for the ratio, because that's the part more difficult to calculate, have to listen to caps, etc.
I noticed that I was normalizing the transformation in my client code. So it makes sense to make the values uniform by default. The patch on my visual editor is also very small: https://github.com/lubosz/gst-gl-tests/commit/509f028db1b27c11f38d88edbdb636b0d626778c The range needs to stay G_MAXFLOAT though, since a scaled / rotated image cannot be moved out of the canvas this way. Z translation should also be greater than -1 in 3D. For the pivot I would suggest a vector, eg pivot-x, pivot-y. Where 0,0 is the default center of the image. (-1,-1) would be one corner of image and (-10,-10) would be outside of the image. The question is if one pivot point should influence rotation and scale, or if 2 pivot points need to be added, so could rotate around one corner but scale at another place. This might be unnecessarily complicated for the user though, but it would allow more use cases.
Created attachment 298146 [details] [review] gltransformation: normalize translations with G_MAXFLOAT range I changed the patch, so the image can be moved out of the canvas when rotated / scaled by increasing the property range to G_MAXFLOAT.
Created attachment 298147 [details] [review] gltransformation: Add pivot point From the commit message: Add an pivot vector for setting the origin of rotations and scales. With the pivot point the rotation and scale operations can have different origins. This adds the ability to rotate around different points. Currently the default (0, 0) pivot point is possible. Rotation around the center, also scaling / zooming into the center. With an pivot point this is optional. I defined the following image coordinates for the pivot point: (-1,1) ----------------- (1,1) | | | | | (0,0) | | | | | | | (-1,-1) ----------------- (1,-1) Example: Rotate the video at the bottom left corner gst-launch-1.0 videotestsrc \ ! gltransformation \ scale-x=0.5 \ scale-y=0.5 \ rotation-z=25.0 \ pivot-x=-1.0 \ pivot-y=-1.0 \ ! glimagesink The pivot-z option defined the pivot point in 3D space, this only affects the rotation, since we have no Z data to scale. With this option a video can be rotated around a point in 3D space. __________ Other examples: Rotate around point behind the image: gst-launch-1.0 videotestsrc ! gltransformation rotation-x=10.0 pivot-z=-4.0 ! glimagesink
Tested those patches, they work nicely in my case. Thanks.
Review of attachment 298147 [details] [review]: Only minor doc and style, I think it's good otherwise. ::: ext/gl/gstgltransformation.c @@ +201,3 @@ + g_object_class_install_property (gobject_class, PROP_PIVOT_X, + g_param_spec_float ("pivot-x", "X Pivot", + "Rotation pivot point X coordinate, where 0 is the center," Maybe "X coordinate of the rotation pivot point" @@ +202,3 @@ + g_param_spec_float ("pivot-x", "X Pivot", + "Rotation pivot point X coordinate, where 0 is the center," + " -1 the lef +1 the right boarder and <-1, >1 outside.", s/lef/left @@ +208,3 @@ + g_object_class_install_property (gobject_class, PROP_PIVOT_Y, + g_param_spec_float ("pivot-y", "Y Pivot", + "Rotation pivot point X coordinate, where 0 is the center," Y, and maybe rephrase to. @@ +209,3 @@ + g_param_spec_float ("pivot-y", "Y Pivot", + "Rotation pivot point X coordinate, where 0 is the center," + " -1 the lef +1 the right boarder and <-1, >1 outside.", Left @@ +280,3 @@ graphene_vec3_init (&up, 0.f, 1.f, 0.f); + // Translate into pivot origin C style comment please (warning only once)
Review of attachment 298146 [details] [review]: Looks good.
Review of attachment 297238 [details] [review]: We'll take the other one ;-P
Review of attachment 297237 [details] [review]: Good.
Review of attachment 297237 [details] [review]: commit 80f97eaee707d7b84493fc919a7cca47887b9d93 Author: Xavier Claessens <xavier.claessens@collabora.com> Date: Wed Feb 18 20:41:14 2015 -0500 gltransformation: Fix doc typo https://bugzilla.gnome.org/show_bug.cgi?id=744763
Review of attachment 298146 [details] [review]: commit 128c2a141aa8772c7b84a93fc022aaf360948c70 Author: Xavier Claessens <xavier.claessens@collabora.com> Date: Wed Feb 18 21:21:01 2015 -0500 gltransformation: normalize translations https://bugzilla.gnome.org/show_bug.cgi?id=744763 * Lubosz: use maxfloat for transformation range
Created attachment 298531 [details] [review] Add pivot point Fixed comment style, fixed typos in commit message.
Review of attachment 298531 [details] [review]: This patch does not apply any-more, would you mind updating ?
Are we still interested in the pivot, or shall be mark this bug as resolved ?
I don't need it in my use cases, so I've got no objection to drop that patch, unless someone needs it?
Created attachment 302976 [details] [review] update pivot patch to current master
I think the addition of a pivot option is useful, also in visual editors. With this patch the user can rotate the image around other places than the center.