GNOME Bugzilla – Bug 651116
CheeseAvatarChooser: add API to specifiy the min and max dimension of the avatar
Last modified: 2020-11-12 07:12:15 UTC
Some IM protocols have requierement on the minimal and maxixum dimension of the avatar. It would be good to be able to pass those to the avatar chooser so user wouldn't be able to choose an avatar not fitting these dimensions.
I think that flexibility is important to the user. Yes we can recommend an avatar size but not make a constraint out of it. If you want we can make the default crop area the recommended one but not let it be the only option for the user.
I think it makes sense to add the constraint. The api can allow specifying minimal and maximun dimensions or leaving it free and each application can use cheese-avatar-chooser as it fits. The key here is that the application is the one part that knows if the constraint is needed or not, not the user.
Created attachment 189580 [details] [review] min_size_property_added_for_crop_area
I added a minimum width and height property for the crop area. If it is ok I will add also a default size and a maximum one.
Review of attachment 189580 [details] [review]: I think you forgot to add the patch for um-crop-area.*. IMHO if the min-height is not set, it should default to zero, meaning no minimum requirement, and it would work just like it was working before. Setting it to 0 should also mean no minimum. An even better idea would be to request the minimum size when calling the get_picture method. I'm thinking of cheese_avatar_chooser_get_constrained_picture (chooser, min_width, min_height), keeping the get_picture as it is.
No, I didn't. The functionality is already implemented in um-crop-area: http://git.gnome.org/browse/cheese/tree/libcheese/um-crop-area.c#n796 I don't think you're right about the calling crop-area-set-min-size in a get-constrained-picture method: we need to set the min size before the user is presented with the chooser window, so the crop-area blocks the user from cropping the image to a very small size. If we do it as you say, then we create the chooser window, we don't set any constraint on the crop-area and the user is able to choose an arbitrary sized crop. We call get-picture only after the chooser-dialog has closed (after the user has successfully taken the picture and cropped it to a desired size). See here where we call get-picture: http://git.gnome.org/browse/empathy/tree/libempathy-gtk/empathy-avatar-chooser.c#n847 As you see, it's after the dialog has been closed and returned the GTK_RESPONSE_ACCEPT status code.
Yes, you're completely right, sorry. So, the nicest way would be to accept the properties on _new instead. But just add the properties as you did, maybe not calling set_min_size if they are not set and add the others as well. We can change _new later if we want and this will solve empathy's case for now. I'm also a little worried that it might be hard to select an avatar respecting the max size if the resolution is not low enough. Do you think we can change that in a smart way just using the requested size if it's done before creating the window?
As you can see the limit on crop-area is set in um-crop-area's init function: http://git.gnome.org/browse/cheese/tree/libcheese/um-crop-area.c#n719 So we had the limits on cheese-avatar-chooser's init implicitly set since before this. I initialized min_width and min_height here for two reasons: a) it's more clear this way how the limit on the crop-area gets set from avatar chooser b) the code for cheese_widget_photo_taken_cb() is simpler if we just use the already initialized min_size/min_width and don't have to bother with checking for valid values (but that's just a if (priv->min_width != -1 and priv->min_height != -1) check, so not very much to gain. Again: the um-crop-area already had min-width and min-height set in it's constructor (and those were set to 48 (hardcoded)). I wonder if other users of libcheese may depend on that functionality.
I'm not sure about max-size and I don't think it's a very real problem given today's cameras. It could be done having two images: one showing the whole image area and one showing only a zoomed component in which the user could select a desired portion. Another approach would be to have scrollbars for the image container and present a zoomed in part of the image. The zoom level would be as big as necessary to make selection of the image at max-size easy. I really don't think it's a real problem and would complicate the code/UI uselessly, but it's your call.
Oh, you were right about a missing patch for um-crop-area: I had these changed in .c: -area->priv->base_width = 48; -area->priv->base_height = 48; +area->priv->base_width = UM_CROP_AREA_MIN_WIDTH; +area->priv->base_height = UM_CROP_AREA_MIN_HEIGHT; and these in .h: +#define UM_CROP_AREA_MIN_WIDTH 48 +#define UM_CROP_AREA_MIN_HEIGHT 48 I'll post this second patch later today when I get back from school.
Ok, so with that part, since not setting min doesn't change anything, patch is good for me. For max size I'm thinking another option would be to just restrain the max size in proportion and convert later too. But let's just add the other properties as simple as we can and make changes if it's necessary later. For now cheese-avatar-chooser uses the resolution cheese is using, right?
Created attachment 190204 [details] [review] um-crop-area: add defines for UM_CROP_AREA_MIN_WIDTH/HEIGHT
Created attachment 190205 [details] [review] libcheese: add min width/height to cheese avatar chooser Updated to make a warning go away because of a missing cast to UM_CROP_AREA().
Created attachment 190206 [details] [review] libcheese: add cheese_avatar_chooser_set_min_size Add new helper function to supplement the gobject properties similar to the one from um-crop-area.
(In reply to comment #0) > Some IM protocols have requierement on the minimal and maxixum dimension of the > avatar. It would be good to be able to pass those to the avatar chooser so user > wouldn't be able to choose an avatar not fitting these dimensions. Seems to me that you should resize it to the user. I can understand having a maximum aspect ratio for example, but having a minimum or maximum _size_ doesn't make sense.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports and feature requests in GNOME Bugzilla which have not seen updates for a long time. If you still use cheese and if you still see this bug / want this feature in a recent and currently supported version, then please feel free to report it at https://gitlab.gnome.org/GNOME/cheese/-/issues/ Thank you for creating this report and we are sorry it could not be implemented (volunteer workforce and time is quite limited).