On 08.03.2016 11:01, Pekka Paalanen wrote: > 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 >
Hi, I've modified the patch to use fixed width for the clock widget. 190 for clock format with seconds, 170 (same as current) for the one without seconds. But in doing so, I had to somehow forward the clock format value to the panel resize handler, which is why new panel structure member was introduced. Thanks for all the feedback so far. Armin
signature.asc
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
