On Mon, 7 Mar 2016 15:42:45 +0100 Armin Krezović <[email protected]> wrote:
> On 07.03.2016 13:37, Pekka Paalanen wrote: > > On Thu, 3 Mar 2016 11:39:13 -0800 > > Bryce Harrington <[email protected]> wrote: > > > >> On Thu, Mar 03, 2016 at 03:48:03PM +0100, Armin Krezović wrote: > >>> This patch enhances the panel clock by adding a config file > >>> option which can be used to either disable the clock or make > >>> it also show seconds in the current clock format. > >>> > >>> v2: Implement suggestions from Pekka's review. > >>> > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57583 > >>> Signed-off-by: Armin Krezović <[email protected]> > >> > >> I might name the enum elements CLOCK_FORMAT_* but as long as it's kept > >> local to the desktop-shell.c client it probably doesn't matter. > >> > >> Reviewed-by: Bryce Harrington <[email protected]> > >> > >> (I just glossed over the adjustments to the cairo position mathematics. > >> I assume it'll be pretty obvious visually if there's an error with > >> that.) > > > > Hi Armin, > > > > there is some funny logic that I'd like to see fixed, details inline. > >>> @@ -368,12 +373,21 @@ panel_clock_redraw_handler(struct widget *widget, > >>> void *data) > >>> CAIRO_FONT_WEIGHT_NORMAL); > >>> cairo_set_font_size(cr, 14); > >>> cairo_text_extents(cr, string, &extents); > >>> - cairo_font_extents (cr, &font_extents); > >>> - cairo_move_to(cr, allocation.x + 5, > >>> + cairo_font_extents(cr, &font_extents); > >>> + > >>> + if (!clock->size_set) { > >>> + allocation.x = allocation.x + allocation.width - extents.width; > >>> + allocation.width = extents.width + 1; > >>> + widget_set_allocation(widget, allocation.x, allocation.y, > >>> + allocation.width, allocation.height); > > > > The call to widget_set_allocation() belongs in the resize function, not > > here in the redraw function. Did you have problems putting it there? > > > > When we had a conversation on IRC about this one, you said that the > proper way to fix this is to get the length from text extents, which > I did. Hi Armin Yeah, sorry, I tend to talk about things unrelated to the immediate topic in question, to give a more broader view and background, and then just leave it to the listener to figure out what's actually in scope for the immediate task. :-) Using extents of the longest possible time string for a format would be the proper way to allocate the widget size, but the topic here is just about adding new formats rather than fixing everything. > I didn't find a way to get access to text extents from within panel > resize function, which is ran before cairo_text_extents is in this > function. Getting the extents in the resize function is indeed the key to the proper sizing solution, which also makes it a little harder to do. That's what my last comment in the bugzilla about extents refers to. It's also a thing one might not bother to do without a reason, like a configurable font or some user setup where the constants are wrong - or another GSoC applicant wanting to prove his skills. Thanks, pq
pgp9we02JmtHe.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
