GNOME Bugzilla – Bug 625959
geometrictransform: make CircleGeometricTransform "radius" property relative
Last modified: 2010-08-04 23:33:13 UTC
See the commit message of the attached patch to understand what it does. The idea is that with absolute radius the user has to always know the video size to get a predictable result while this way the user who knows the video size can calculate the relative radius and set it and the one who doesn't can just set a the relative one and be happy. Also makes "radius" more consistent with "x-center" and "y-center" properties. Currently the radius is defined with respect to the circumscribed circle because it seemed more intuitive to me. The inscribed circle would probably be good too, I guess it's a matter of personal preferences, Tell me what you like more.
Created attachment 167058 [details] [review] geometrictransform: make ciclegt "radius" property relative Make the "radius" property of CircleGeometricTransform relative. This is more coherent with the way [x,y]-center properties are handled and allow to set a radius without knowing the video size. Radius is defined with respect to the circle circumscribed about the video rectangle so that a point in the center has radius 0.0 and one in a vertex has radius 1.0. Note that this is not a regression from the previous absolute way of defining the radius as a user who knows the video size can easily calculate the relative radius and set that.
Review of attachment 167058 [details] [review]: Found one suspicious change, the others looks fine. ::: gst/geometrictransform/gstcirclegeometrictransform.c @@ +159,3 @@ + cgt->radius * sqrt (0.5 * (gt->width * gt->width + + gt->height * gt->height)); + cgt->precalc_radius2 = cgt->precalc_radius * cgt->precalc_radius; Shouldn't the 0.5 be outside the sqrt? the sqrt calculates the diagonal length, and you split it in half to get the maximum radius, right?
(In reply to comment #2) > Review of attachment 167058 [details] [review]: > > Found one suspicious change, the others looks fine. > > ::: gst/geometrictransform/gstcirclegeometrictransform.c > @@ +159,3 @@ > + cgt->radius * sqrt (0.5 * (gt->width * gt->width + > + gt->height * gt->height)); > + cgt->precalc_radius2 = cgt->precalc_radius * cgt->precalc_radius; > > Shouldn't the 0.5 be outside the sqrt? > > the sqrt calculates the diagonal length, and you split it in half to get the > maximum radius, right? Yup :-P you're right! Correct patch following.
Created attachment 167074 [details] [review] geometrictransform: make ciclegt "radius" property relative Make the "radius" property of CircleGeometricTransform relative. This is more coherent with the way [x,y]-center properties are handled and allow to set a radius without knowing the video size. Radius is defined with respect to the circle circumscribed about the video rectangle so that a point in the center has radius 0.0 and one in a vertex has radius 1.0. Note that this is not a regression from the previous absolute way of defining the radius as a user who knows the video size can easily calculate the relative radius and set that.
This blocks #625908 I guess
Module: gst-plugins-bad Branch: master Commit: c921067208414c47a0c961e604b4d777f2d40b34 URL: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=c921067208414c47a0c961e604b4d777f2d40b34 Author: Filippo Argiolas <filippo.argiolas@gmail.com> Date: Tue Aug 3 10:08:34 2010 +0200 geometrictransform: make ciclegt "radius" property relative Make the "radius" property of CircleGeometricTransform relative. This is more coherent with the way [x,y]-center properties are handled and allow to set a radius without knowing the video size. Radius is defined with respect to the circle circumscribed about the video rectangle so that a point in the center has radius 0.0 and one in a vertex has radius 1.0. Note that this is not a regression from the previous absolute way of defining the radius as a user who knows the video size can easily calculate the relative radius and set that. https://bugzilla.gnome.org/show_bug.cgi?id=625959