Bug 52454

Summary: [bug] Weston tooltips are incorrectly cropped
Product: Wayland Reporter: Brian Lovin <brian.j.lovin>
Component: westonAssignee: Wayland bug list <wayland-bugs>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Cropping incorrect, double mouse cursors are an artifact of the screenshot process and can be ignored.
patch

Description Brian Lovin 2012-07-24 17:27:03 UTC
Created attachment 64625 [details]
Cropping incorrect, double mouse cursors are an artifact of the screenshot process and can be ignored.

System Environment:
--------------------------
Distro: Ubuntu 12.04
Arch: x86_64
glproto master ec1eec4355ee4a6c5134f2178192f10b6d28a87a
mesa master 82fc813ca870c4002502e098519bead7bec1a7e8
wayland master eadb68ec32df8ee54ccba175d19adcdedfaa5015
pixman master 56321eff65832791252c7c324930d14c44d4d5f7
weston master 2327d1f490a5362e6b6199ca68b14d11e2f13408
drm master ebd7904877d08525beb5039e4ea2f5b6c0a7c23f
cairo master 21e3f2e9034b64131075d82a4e34868dc72f2249
libX11 master 8042f88ace33573f9d0dfaa21ed54ac7cef266d5
macros master 0890e4003aacfa7113ab3f4e3ad7c5636f8e922a
libxkbcommon master fe4f990902c7cd8f395b9f3be14dc6e8cad7cae9
dri2proto master 4eeacce4c4a300b938b7e3fb78a8e443c491780b
kbproto master 391a1f6de6315fc0196d407d800597488315ccc

Detailed Description:
-----------------------------
Run weston and then mouse over one of the launcher application icons.
Observe when the tooltip pops up it is cropped incorrectly - the right side is cut off.

See photo for reference, once again the smaller mouse pointer is not observed when running the test.

Steps to Reproduce:
----------------------------
1. Start weston under X11 or DRM mode
2. Mouse over launcher icon
3. Observe the tooltip not showing all the text and being cropped incorrectly
Comment 1 Tiago Vignatti 2012-07-30 08:34:28 UTC
That's because tooltips need cairo EGL to get text extents (HAVE_CAIRO_EGL in config.h).
Comment 2 Kristian Høgsberg 2012-10-30 00:26:00 UTC
(In reply to comment #1)
> That's because tooltips need cairo EGL to get text extents (HAVE_CAIRO_EGL
> in config.h).

Tooltips should work with both EGL and sw cairo.
Comment 3 U. Artie Eoff 2012-10-30 00:58:06 UTC
I'm still able to reproduce this issue using cairo-image on today's master bits (I have not tried with cairo-egl).
Comment 4 Scott Moreau 2012-10-30 02:30:10 UTC
Created attachment 69268 [details] [review]
patch

(In reply to comment #3)
> I'm still able to reproduce this issue using cairo-image on today's master
> bits (I have not tried with cairo-egl).

I was able to reproduce this with --with-cairo-glesv2 and decided to look into it. The problem is that window->cairo_surface is NULL when get_text_extents() is called because this surface is destroyed on attach, for the shm path. I've attached a patch to fix the problem. I'm not sure if it's correct since I'm not sure why cairo_surface is destroyed on the shm path only.
Comment 5 Kristian Høgsberg 2012-10-30 14:02:24 UTC
(In reply to comment #4)
> Created attachment 69268 [details] [review] [review]
> patch
> 
> (In reply to comment #3)
> > I'm still able to reproduce this issue using cairo-image on today's master
> > bits (I have not tried with cairo-egl).
> 
> I was able to reproduce this with --with-cairo-glesv2 and decided to look
> into it. The problem is that window->cairo_surface is NULL when
> get_text_extents() is called because this surface is destroyed on attach,
> for the shm path. I've attached a patch to fix the problem. I'm not sure if
> it's correct since I'm not sure why cairo_surface is destroyed on the shm
> path only.

Yeah, I think this is a good fix.  I thought we relies on the surface being destroyed after flush, but that's obviously not how the EGL surface works.  Your patch brings the two backends closer in how they work and fix the problem... let's go with that.  Mind sending it to the list?
Comment 6 Scott Moreau 2012-10-30 18:12:30 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Created attachment 69268 [details] [review] [review] [review]
> > patch
> > 
> > (In reply to comment #3)
> > > I'm still able to reproduce this issue using cairo-image on today's master
> > > bits (I have not tried with cairo-egl).
> > 
> > I was able to reproduce this with --with-cairo-glesv2 and decided to look
> > into it. The problem is that window->cairo_surface is NULL when
> > get_text_extents() is called because this surface is destroyed on attach,
> > for the shm path. I've attached a patch to fix the problem. I'm not sure if
> > it's correct since I'm not sure why cairo_surface is destroyed on the shm
> > path only.
> 
> Yeah, I think this is a good fix.  I thought we relies on the surface being
> destroyed after flush, but that's obviously not how the EGL surface works. 
> Your patch brings the two backends closer in how they work and fix the
> problem... let's go with that.  Mind sending it to the list?

Sure, the patch has been sent to the list.
Comment 7 Kristian Høgsberg 2012-10-30 18:21:19 UTC
Thanks!

commit 1bdb477522ee364ca660064e8ca232fbdece09f6
Author: Scott Moreau <oreaus@gmail.com>
Date:   Tue Oct 30 12:12:12 2012 -0600

    toytoolkit: Don't destroy window cairo surface on shm attach.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=52454
Comment 8 Brian Lovin 2012-11-27 23:07:18 UTC
Changing to verified

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.