Merge lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-dfdt-factors-fix into lp:ubuntu-ui-toolkit/staging

[Loïc Molinari]

[UbuntuShape] Computed dfdt factors based on window content orientation.

Previous logic was based on screen orientation which is not in sync with OrientationHelper.qml.

[UbuntuShape] Computed dfdt factors based on window content orientation.

Previous logic was based on screen orientation which is not in sync with OrientationHelper.qml.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1188
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1778/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2905
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/506
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/508
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/508/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/505
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2491
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2903
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2903/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20677

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1778/rebuild

review: Needs Fixing (continuous-integration)

[lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-dfdt-factors-fix]

1189. By Loïc Molinari on 2015-05-21

Explicitely initialized .bss variable for portability reasons (not required by C standard but set to 0 on Linux for sure).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1189
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1784/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2914
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/512
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/514
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/514/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/511
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2498
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2912
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2912/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20689

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1784/rebuild

review: Needs Fixing (continuous-integration)

[Christian Dywan (kalikiana)]

FYI I tried this in conjunction with my branch for orientation test cases and buttons are still jagged badly around the border, for your convenience a throw-away test branch (./run_tests.sh ubuntuuitoolkit.tests.components.test_popover.PopoverRotateTestCase.test_rotate to see it in action).

[Christian Dywan (kalikiana)]

As discussed, this fix specifically addresses aliasing seen when rotating a device with an app locked to original orientation (such as toolkit gallery when modified to not rotate) and it works perfectly.

The symptom is exactly the same as what I referred to above, but that's visible in tests only, and can be addressed in a follow-up in any case.

My final concern here is actually that this is very hard to verify as long as very different specific conditions lead to aliasing issues. And multi monitor support will make it even more complex. I filed bug 1461077.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/plugin/ucubuntushape.cpp'
--- modules/Ubuntu/Components/plugin/ucubuntushape.cpp	2015-04-30 19:51:26 +0000
+++ modules/Ubuntu/Components/plugin/ucubuntushape.cpp	2015-05-21 11:25:23 +0000
@@ -49,36 +49,8 @@
 
 // --- Scene graph shader ---
 
-// Factors used to know which screen-space derivatives must be used in the fragment shaders based on
-// the primary orientation and current orientation.
-static float dfdtFactors[2];
-
-static void orientationChanged(Qt::ScreenOrientation orientation)
-{
-    const quint8 landscapeMask = Qt::LandscapeOrientation | Qt::InvertedLandscapeOrientation;
-    const quint8 portraitMask = Qt::PortraitOrientation | Qt::InvertedPortraitOrientation;
-    if (QGuiApplication::primaryScreen()->primaryOrientation() & landscapeMask) {
-        const quint8 flippedMask =
-            Qt::InvertedLandscapeOrientation | Qt::InvertedPortraitOrientation;
-        dfdtFactors[0] = orientation & landscapeMask ? 0.0f : 1.0f;
-        dfdtFactors[1] = orientation & flippedMask ? -1.0f : 1.0f;
-    } else {
-        const quint8 flippedMask = Qt::InvertedPortraitOrientation | Qt::LandscapeOrientation;
-        dfdtFactors[0] = orientation & portraitMask ? 0.0f : 1.0f;
-        dfdtFactors[1] = orientation & flippedMask ? -1.0f : 1.0f;
-    }
-}
-
 ShapeShader::ShapeShader()
 {
-    static bool once = true;
-    if (once) {
-        const QScreen* primaryScreen = QGuiApplication::primaryScreen();
-        orientationChanged(primaryScreen->orientation());
-        QObject::connect(primaryScreen, &QScreen::orientationChanged, orientationChanged);
-        once = false;
-    }
-
     setShaderSourceFile(QOpenGLShader::Vertex, QStringLiteral(":/uc/shaders/shape.vert"));
     setShaderSourceFile(QOpenGLShader::Fragment, QStringLiteral(":/uc/shaders/shape.frag"));
 }
@@ -168,9 +140,11 @@
 
     // Send screen-space derivative factors. Note that when rendering is redirected to a
     // ShaderEffectSource (FBO), dFdy() sign is flipped.
-    const bool flipped = dfdtFactors[0] != 1.0f && state.projectionMatrix()(1, 3) < 0.0f;
-    const QVector2D dfdtFactorsVector(dfdtFactors[0], flipped ? -dfdtFactors[1] : dfdtFactors[1]);
-    program()->setUniformValue(m_dfdtFactorsId, dfdtFactorsVector);
+    const float orientation = static_cast<float>(data->dfdtFactors & 0x4);
+    const float flip = static_cast<float>(data->dfdtFactors & 0x3) - 1.0f;
+    const bool flipped = orientation != 1.0f && state.projectionMatrix()(1, 3) < 0.0f;
+    const QVector2D dfdtFactors(orientation, flipped ? -flip : flip);
+    program()->setUniformValue(m_dfdtFactorsId, dfdtFactors);
 
     // Update QtQuick engine uniforms.
     if (state.isMatrixDirty()) {
@@ -272,6 +246,7 @@
 // --- QtQuick item ---
 
 static QHash<QOpenGLContext*, quint32> shapeTextureHash;
+static bool isPrimaryOrientationLandscape = false;
 
 const float implicitWidthGU = 8.0f;
 const float implicitHeightGU = 8.0f;
@@ -331,6 +306,17 @@
     , m_sourceOpacity(255)
     , m_flags(Stretched)
 {
+    static bool once = true;
+    if (once) {
+        // Stored statically as the primary orientation is fixed and we don't support multiple
+        // screens for now.
+        if (QGuiApplication::primaryScreen()->primaryOrientation() &
+            (Qt::LandscapeOrientation | Qt::InvertedLandscapeOrientation)) {
+            isPrimaryOrientationLandscape = true;
+        }
+        once = false;
+    }
+
     setFlag(ItemHasContents);
     QObject::connect(&UCUnits::instance(), SIGNAL(gridUnitChanged()), this,
                      SLOT(_q_gridUnitChanged()));
@@ -1236,6 +1222,25 @@
     materialData->distanceAAFactor = qMin(
         (radius / (end - start)) - (start / (end - start)), 1.0f) * 255.0f;
 
+    // Screen-space derivatives factors for fragment shaders depend on the primary orientation and
+    // content orientation. A flag indicating a 90° rotation around the primary orientation is
+    // stored on the 3rd bit of dfdtFactors, the flip factor is stored on the first 2 bits as 0 for
+    // -1 and as 2 for 1 (efficiently converted using: float(x & 0x3) - 1.0f).
+    const Qt::ScreenOrientation contentOrientation = window()->contentOrientation();
+    if (isPrimaryOrientationLandscape) {
+        const quint8 portraitMask = Qt::PortraitOrientation | Qt::InvertedPortraitOrientation;
+        const quint8 flipMask = Qt::InvertedLandscapeOrientation | Qt::InvertedPortraitOrientation;
+        quint8 factors = contentOrientation & portraitMask ? 0x4 : 0x0;
+        factors |= contentOrientation & flipMask ? 0x0 : 0x2;
+        materialData->dfdtFactors = factors;
+    } else {
+        const quint8 landscapeMask = Qt::LandscapeOrientation | Qt::InvertedLandscapeOrientation;
+        const quint8 flipMask = Qt::InvertedPortraitOrientation | Qt::LandscapeOrientation;
+        quint8 factors = contentOrientation & landscapeMask ? 0x4 : 0x0;
+        factors |= contentOrientation & flipMask ? 0x0 : 0x2;
+        materialData->dfdtFactors = factors;
+    }
+
     // When the radius is equal to radiusSizeOffset (which means radius size is 0), no aspect is
     // flagged so that a dedicated (statically flow controlled) shaved off shader can be used for
     // optimal performance.

=== modified file 'modules/Ubuntu/Components/plugin/ucubuntushape.h'
--- modules/Ubuntu/Components/plugin/ucubuntushape.h	2015-04-22 19:33:39 +0000
+++ modules/Ubuntu/Components/plugin/ucubuntushape.h	2015-05-21 11:25:23 +0000
@@ -68,6 +68,7 @@
         quint32 shapeTexture;
         quint8 distanceAAFactor;
         quint8 sourceOpacity;
+        quint8 dfdtFactors;
         quint8 flags;
     };