Opened 4 years ago

Closed 4 years ago

#12542 closed defect (fixed)

Screen rotation causes graphics corruption on XO-4

Reported by: greenfeld Owned by: dsd
Priority: high Milestone: 13.2.0
Component: x window system Version: Development build as of this date
Keywords: Cc:
Blocked By: Blocking:
Deployments affected: Action Needed: test in build
Verified: no

Description

Rotating the screen 360 degrees (four times using the rotate key) and then launching an application like Scratch or Record usually leads to graphics corruption.

In the case of Record, the Xv surface which should show the camera may show alternative bitmaps instead.

Seen in 13.1.0 os28/29 {with the dove driver}.

Attachments (1)

moon.py (2.7 KB) - added by dsd 4 years ago.
moon.py program used in the test

Download all attachments as: .zip

Change History (19)

comment:1 Changed 4 years ago by greenfeld

  • Summary changed from Screen rotation causes graphics corruption to Screen rotation causes graphics corruption on XO-4

comment:2 Changed 4 years ago by erikos

#12610 has info about occasions in Moon, Memorize and the sharing Palette. Please make sure all those cases got fixed.

comment:3 Changed 4 years ago by dsd

  • Priority changed from normal to high

comment:4 Changed 4 years ago by godiard

The touch cursors on gtk3 entries show artifacts too.

comment:5 Changed 4 years ago by dsd

I think the fix we shipped for XO-1.75 in the past was: http://lists.x.org/archives/xorg-devel/2011-June/023008.html

comment:7 Changed 4 years ago by dsd

  • Milestone changed from 13.1.0 to 13.2.0
  • Owner changed from jnettlet to dsd

comment:8 Changed 4 years ago by dsd

Fixed in xorg-x11-server-1.13.3-1.fc18.olpc3

Will leave this open while we get some upstream communication going...

comment:10 Changed 4 years ago by dsd

Chris Wilson thinks this is a problem in the dove DDX. To get more of a grasp on the problem, here is an audit of the CreatePixmap2, ModifyPixmapHeader and DestroyPixmap calls that arrive in the driver, when running an X session which does the following.

  1. Start X
  2. xrandr -o left
  3. xrandr -o normal
  4. Run "moon.py" (program that draws a pixmap of a moon image on screen, exits after a couple of seconds)
  5. Close X

Upon X init

  • CreatePixmap w=0 h=0 (note that this pixmap is never destroyed)
  • ModifyPixmapHeader on that pixmap, w=0 h=0
  • ModifyPixmapHeader on that pixmap, w=1200 h=1200
  • Create 3 internal 1200x900 pixmaps used for EXA operations (one for each of alpha, mask, repeat)
  • CreatePixmap w=16 h=16
  • ModifyPixmapHeader on that pixmap, w=16 h=16 (note that this pixmap is never destroyed)
  • CreatePixmap w=16 h=16
  • ModifyPixmapHeader on that pixmap, w=16 h=16
  • DestroyPixmap on that pixmap
  • CreatePixmap w=16 h=16
  • ModifyPixmapHeader on that pixmap, w=16 h=16
  • DestroyPixmap on that pixmap

Upon xrandr -o left

  • Destroy 3 internal EXA pixmaps
  • Create 3 internal 900x1200 pixmaps used for EXA operations
  • CreatePixmap w=0 h=0 (lets call this one "pixmap A" for later reference)
  • ModifyPixmapHeader on that pixmap, w=0 h=0
  • ModifyPixmapHeader on that pixmap, w=1200 h=900

Upon xrandr -o normal

  • Destroy 3 internal EXA pixmaps
  • Create 3 internal 1200x900 pixmaps used for EXA operations (these pixmaps are never destroyed, but this is clearly an internal/unrelated driver bug)

Upon launching moon.py

  • CreatePixmap w=1 h=1
  • ModifyPixmapHeader on that pixmap, w=1 h=1
  • CreatePixmap w=726 h=726
  • ModifyPixmapHeader on that pixmap, w=726 h=726
  • ModifyPixmapHeader on pixmap A, w=1536 h=64, happens 37 times at this point
  • CreatePixmap w=726 h=726
  • DestroyPixmap on that pixmap
  • DestroyPixmap on the 1x1 pixmap that was created when moon.py was launched
  • DestroyPixmap on the 726x726 pixmap that was created when moon.py was launched

Upon X shutdown

  • DestroyPixmap on pixmap A

Observations

The very first pixmap created (1200x1200) is never destroyed. A 16x16 pixmap also created during X init is never destroyed.

There are 37 consecutive calls to ModifyPixmapHeader that happen at the same time when the image is being drawn on screen, I assume that corresponds to the drawing of the image. I am a little concerned that the pixmap in question here was created when the screen was rotated left (we have since returned to normal orientation) - it just seems a little odd to be using that "stale" pixbuf (whereas using one that was freshly created or one that was created before any rotation happened would make a bit more sense to me).

comment:11 Changed 4 years ago by dsd

To clarify, the above audit is from an X server without Chris Wilson's patch applied (i.e. the moon was rendered badly).

Changed 4 years ago by dsd

moon.py program used in the test

comment:12 Changed 4 years ago by dsd

In the above analysis, "pixmap A" is actually the scratch pixmap. I confirmed this by looking at the code more closely. At the point when "pixmap A" is allocated, we are in mrvl_crtc_shadow_create() which does basically:

    return GetScratchPixmapHeader(pScrn->pScreen,
                                           width, height,
                                           pScrn->depth,
                                           pScrn->bitsPerPixel,
                                           rotate_pitch,
                                           data);

When we get to mrvl_crtc_shadow_destroy() we do the opposite, FreeScratchPixmapHeader, but in the current unpatched X code, that function doesn't actually do any freeing, it keeps the pixmap around for later reuse, so DestroyPixmap is not called.

Then later when we come to draw the moon, the exa code calls GetScratchPixmapHeader() which causes reuse of the scratch pixmap (rather than allocating a new one), and it is the re-use that seems to lead to corrupted drawing. (but I am not exactly sure why)

If I modify the test to just start X, draw the moon, and exit, I can see that the scratch pixmap is not created right until the point when drawing happens.

The same bug seems to have hit geode in the past: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-geode/+bug/377929/comments/24 - and the solution chosen there is to not use the scratch pixmap for rotation purposes. Maybe we should do the same.

comment:13 Changed 4 years ago by dsd

  • Action Needed changed from diagnose to add to build

Yes, lets take the geode approach. Fixed in xorg-x11-drv-dove-0.3.7 and the X patch has been dropped from xorg-x11-server-1.13.3-1.fc18.olpc4.

comment:14 Changed 4 years ago by erikos

When I rotate the screen with xorg-x11-drv-dove 3.6 and 3.7 and the Record activity is open I see parts of the Home screen or a grey area, is that another remaining issue?

comment:15 follow-up: Changed 4 years ago by dsd

But the display does sort itself out after a second or two?

That is not a new issue as far as I can see.

comment:16 in reply to: ↑ 15 Changed 4 years ago by erikos

Replying to dsd:

But the display does sort itself out after a second or two?

That is not a new issue as far as I can see.

Yes, the display does sort itself out. I thought we had fixed this at some point. But maybe I am wrong. I could not find an old bug about it, should we open a new ticket for this?

comment:17 Changed 4 years ago by dsd

  • Action Needed changed from add to build to test in build

Test in 13.2.0 build 5

comment:18 Changed 4 years ago by dsd

  • Resolution set to fixed
  • Status changed from new to closed

Tested 13.2.0 build 5 for XO-4, rotating the screen 4 times then opening Moon no longer produces bad graphics.

Note: See TracTickets for help on using tickets.