Default blacklist string shouldn't contain double escape

Bug #1091103 reported by Timo Jyrinki
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Compiz
Fix Released
Undecided
Timo Jyrinki
0.9.8
Fix Released
Undecided
Timo Jyrinki
Compiz Core
Fix Released
Undecided
Timo Jyrinki
compiz (Ubuntu)
Fix Released
Undecided
Timo Jyrinki
Nominated for Quantal by Timo Jyrinki
Precise
Fix Released
Undecided
Timo Jyrinki

Bug Description

Note: similar to bug #1089246, this SRU only affects precise since on quantal/raring it wouldn't have an effect.

[Impact]

Incorrect default setting doesn't have the wanted effect of functional blacklist on precise, because of character escaping problems.

[Test Case]

The test case relies on a bug/feature of Intel driver that on modern Intel hardware like sandy bridge / ivy bridge, tear-free video playback is not possible without compositing if the video player doesn't use vsync itself. Thus tearing can be seen as an indication if unredirection is in effect or not.

0. Have intel hardware on Ubuntu 12.04 LTS, with which this is easiest to test
1. In CCSM, set Composite -> unredirect_match to '(any)', ie. remove everything that comes after that, that would normally prevent unredirection in common video players.
2. Open http://www.youtube.com/watch?v=ZCPkOpMHB7g in Firefox and play it full screen
3. Expected result: no tearing, because blacklist is in effect and prevents unredirection on Intel (and nouveau) regardless of the match setting

Note: radeon always has tearfree Xv output, so you can't use a similar test and changing the blacklist string to include 'AMD' without forcing non-Xv X11 output

[Regression Potential]

Very low. Theoretically too excessive blacklisting could restore current precise behavior.

---

The blacklist default option in a true precise environment - the only environment it's supposed to have an effect with its default setting - is not working. It seems that the default unredirect_driver_blacklist in the XML contains the same double escape as the test code. So the tests succeed, but in reality there is extra '\' in the default setting, preventing the regexp from working.

This one, the current default doesn't work:
(nouveau|Intel).*Mesa 8\\.0

Either one of these works instead:
(nouveau|Intel).*Mesa 8\.0
(nouveau|Intel).*Mesa 8.0

Related branches

description: updated
description: updated
description: updated
Changed in compiz:
status: New → Fix Committed
description: updated
Changed in compiz-core:
status: New → Fix Committed
Revision history for this message
Adam Conrad (adconrad) wrote : Please test proposed package

Hello Timo, or anyone else affected,

Accepted compiz into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/compiz/1:0.9.7.12-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-needed
Revision history for this message
Miklos Juhasz (mjuhasz) wrote :

The proposed version fixes this bug. I have executed the testcase on an up-to-date Precise installation using Sandy Bridge.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Colin Watson (cjwatson) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package compiz - 1:0.9.7.12-0ubuntu1

---------------
compiz (1:0.9.7.12-0ubuntu1) precise-proposed; urgency=low

  [ Łukasz 'sil2100' Zemczak ]
  * debian/patches/revert_fix_994841.patch,
    debian/patches/revert_fix_933776_955035.patch
    - Removed, upstreamed
  * debian/watch:
    - Changed to the correct LP tarball path
  * debian/patches/compiz-package-gles2.patch:
    - Updated GLES patches to properly apply to the new compiz version

  [ Timo Jyrinki ]
  * New upstream release.
    - Compiz won't start if "unredirect fullscreen windows" is enabled
      (LP: #980663)
    - "Unredirect Fullscreen Windows" stay on top (unredirected) even
      when they're not on top any more (or the output is transformed)
      (LP: #1041047)
    - Unredirect Fullscreen Windows sometimes fails to unredirect fullscreen
      windows at all (LP: #1041066)
    - "Unredirect Fullscreen Windows" stay on top (unredirected) even when
      an RGBA window is stacked above it (LP: #1046661)
    - scale mode is not visible if a fullscreen window is unredirected
      (LP: #1047168)
    - Unredirecting a fullscreen window on a secondary monitor causes that
      monitor to flicker (LP: #1050749)
    - "Unredirect Fullscreen Windows" makes multi-monitor rendering much
      slower (LP: #1051885)
    - [regression] Maximized window gets unredirected when it's not
      fullscreen (LP: #1053902)
    - Unredirected fullscreen windows freeze and stay on top when wall
      sliding (Ctrl+Alt+Left/Right) (LP: #1084401)
    - "Unredirect Fullscreen Windows" can cause significant tearing on
      fullscreen windows (especially playing video) on some drivers
      (LP: #1051802)
    - HTML5 video in Firefox continues to tear (LP: #1086337)
    - Add support for blacklisting some drivers from using unredirected
      fullscreen windows. By default intel and nouveau on Mesa 8.0
      are blacklisted, configurable via ccsm. Users can upgrade to
      Mesa 9.0 around 12.04.2 time. (LP: #1089246)
    - Unredirect fullscreen windows should be the default for optimal
      performance (LP: #1063690)
  * debian/patches/workaround_broken_drivers.patch:
    - Updated to apply.
  * debian/patches/force_unredirect_enabling.patch:
    - Enable unredirect_fullscreen_windows unconditionally. Because of
      gconf hurdles we can't easily migrate existing users into the
      new default otherwise. Unredirection can be easily disabled with
      the new unredirect_match option, by blanking it completely,
      including removing the '(any)' part.
  * debian/patches/compiz-package-gles2.patch:
    - Update for the new blacklisting feature as well
  * debian/patches/blacklist_no_xml_double_escaping.patch:
    - Cherry-pick fix for the default string (LP: #1091103)

  [ Michael Terry ]
  * debian/patches/blacklist_no_xml_double_escaping.patch:
    - Update to also blacklist Mesa 9.0, which precise users may have from
      the x-updates PPA. 9.0 seems to have similar problems as 8.0 on
      Intel hardware.
 -- Timo Jyrinki <email address hidden> Fri, 14 Dec 2012 10:41:24 +0200

Changed in compiz (Ubuntu Precise):
status: New → Fix Released
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Actually it should be:
  (nouveau|Intel).*Mesa 8\.0
or in areas like C code, quoted as:
  (nouveau|Intel).*Mesa 8\\.0

The solution that was implemented:
  (nouveau|Intel).*Mesa 8.0
is technically wrong because that will match "Mesa 800" or "Mesa "8A0". But it will work well enough for the Mesa versions which exist in reality :)

Changed in compiz (Ubuntu):
status: New → Fix Committed
Changed in compiz:
milestone: none → 0.9.9.0
Changed in compiz-core:
milestone: none → 0.9.7.14
Changed in compiz:
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Changed in compiz-core:
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Changed in compiz (Ubuntu):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Changed in compiz (Ubuntu Precise):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package compiz - 1:0.9.9~daily13.01.14-0ubuntu1

---------------
compiz (1:0.9.9~daily13.01.14-0ubuntu1) raring; urgency=low

  [ sampo555 ]
  * compiz crashed with SIGSEGV in DodgeAnim::applyDodgeTransform() (LP:
    #1048840)
  * compiz crashing if window un-/minimize animation is "Random" (LP:
    #1098185)

  [ Daniel van Vugt ]
  * Several leaks in new GLProgram from compileProgram() from
    GLScreen::getProgram() from GLWindowAutoProgram::getProgram() (LP:
    #1097644)

  [ Sam Spilsbury ]
  * Several leaks in ccsIntegratedSettingListAppend() ... from
    ccsGNOMEIntegrationBackendGetIntegratedSetting() from readSetting
    (gsettings.c:375) (LP: #1097661)

  [ MC Return ]
  * Thumbnail Window Previews: Flickering of background/glow and window
    title text (LP: #1098758)

  [ Automatic PS uploader ]
  * Automatic snapshot from revision 3561
 -- Automatic PS uploader <email address hidden> Mon, 14 Jan 2013 04:03:09 +0000

Changed in compiz (Ubuntu):
status: Fix Committed → Fix Released
Changed in compiz:
status: Fix Committed → Fix Released
Revision history for this message
Stephen M. Webb (bregma) wrote :

marking as closed (0.9.8 series is obsolete)

Changed in compiz-core:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.