Use LCD text if the transform IsAlmostIdentityAndIntegerTranslation

cc/layer_tree_host_common.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layer_tree_host_common.h"
 
 #include <algorithm>
 
 #include "base/debug/trace_event.h"
 #include "cc/heads_up_display_layer_impl.h"
@@ skipping 528 matching lines @@
     layer->drawProperties().num_descendants_that_draw_content = numDescendantsThatDrawContent;
     layer->drawProperties().descendants_can_clip_selves = descendantsCanClipSelves;
 }
 
 static void roundTranslationComponents(gfx::Transform* transform)
 {
     transform->matrix().setDouble(0, 3, MathUtil::Round(transform->matrix().getDouble(0, 3)));
     transform->matrix().setDouble(1, 3, MathUtil::Round(transform->matrix().getDouble(1, 3)));
 }
 
+static void roundAlmostIntegerComponent(SkMatrix44* matrix, int i, int j)
+{
+    const double kErrorThreshold = 0.0001;
+    double d = matrix->getDouble(i, j);
+    double rounded = MathUtil::Round(d);
+    double error = std::abs(rounded - d);
+    if (error > 0 && error < kErrorThreshold)
+        matrix->setDouble(i, j, rounded);
+}
+
+// Tiny errors caused by multiple multiplications and divisions on mixed double
+// and float operands may result almost-integer scales and translations like
+// 1.00001. Round them so that some transform can be identity as expected.
+static void roundAlmostIntegerScalesAndTranslationComponents(gfx::Transform* transform)
+{
+    if (transform->IsScaleOrTranslation() && !transform->IsIdentityOrTranslation()) {

[Ian Vollick, 2013/03/07 21:18:36]

Unless I'm reading this wrong, it looks like we'll opt in if (scale ||
translation) && (!identity && !translation) -- i.e., a straight translation
can't be rounded. Isn't that the the most important case?

+        SkMatrix44* matrix = &transform->matrix();
+        roundAlmostIntegerComponent(matrix, 0, 0);
+        roundAlmostIntegerComponent(matrix, 1, 1);
+        roundAlmostIntegerComponent(matrix, 0, 3);
+        roundAlmostIntegerComponent(matrix, 1, 3);
+    }
+}
+
 // Recursively walks the layer tree starting at the given node and computes all the
 // necessary transformations, clipRects, render surfaces, etc.
 template<typename LayerType, typename LayerList, typename RenderSurfaceType>
 static void calculateDrawPropertiesInternal(LayerType* layer, const gfx::Transform& parentMatrix,
     const gfx::Transform& fullHierarchyMatrix, const gfx::Transform& currentScrollCompensationMatrix,
     const gfx::Rect& clipRectFromAncestor, const gfx::Rect& clipRectFromAncestorInDescendantSpace, bool ancestorClipsSubtree,
     RenderSurfaceType* nearestAncestorThatMovesPixels, LayerList& renderSurfaceLayerList, LayerList& layerList,
     LayerSorter* layerSorter, int maxTextureSize, float deviceScaleFactor, float pageScaleFactor, bool subtreeCanUseLCDText,
     gfx::Rect& drawableContentRectOfSubtree, bool updateTilePriorities)
 {
@@ skipping 153 matching lines @@
         // fixed correctly.
         // Note carefully: this is Concat, not Preconcat (currentScrollCompensation * combinedTransform).
         combinedTransform.ConcatTransform(currentScrollCompensationMatrix);
     }
 
     // The drawTransform that gets computed below is effectively the layer's drawTransform, unless
     // the layer itself creates a renderSurface. In that case, the renderSurface re-parents the transforms.
     layerDrawProperties.target_space_transform = combinedTransform;
     // M[draw] = M[parent] * LT * S[layer2content]
     layerDrawProperties.target_space_transform.Scale(1.0 / layer->contentsScaleX(), 1.0 / layer->contentsScaleY());
+    // Eliminate the tiny errors in scales and translations to prevent the errors from propagating,
+    // to prevent bluriness and ensures LCDText working as expected.
+    roundAlmostIntegerScalesAndTranslationComponents(&layerDrawProperties.target_space_transform);

[enne (OOO), 2013/03/07 19:05:34]

Is it ok to round the draw transform but not the screen space transform
equivalently? Is that dangerous for damage tracking?

[Xianzhu, 2013/03/07 19:38:17]

I think the rounding will reflect in screen_space_transform because it includes
target_space_transform.

[shawnsingh, 2013/03/07 20:13:36]

I agree with Xianzhu, it should be OK with respect to damage tracking, because
the screen space transform is computed from the rounded target space transform. 
I think the damage tracker will work fine as long as it uses the exact same
transforms that are used for drawing.

[Ian Vollick, 2013/03/07 21:18:36]

I'd considered tackling this sort of rounding, too. At that time, I'd thought
that the safest spot was at the last possible moment -- in gl_renderer.cc right
before we hand the matrix off to gl. This would protect us from a mistake where
we modify the screen space transform after rounding it and busting our LCD text.
Is that an option?

 
     // layerScreenSpaceTransform represents the transform between root layer's "screen space" and local content space.
     layerDrawProperties.screen_space_transform = fullHierarchyMatrix;
     if (!layer->preserves3D())
         layerDrawProperties.screen_space_transform.FlattenTo2d();
     layerDrawProperties.screen_space_transform.PreconcatTransform(layerDrawProperties.target_space_transform);
 
     // Adjusting text AA method during animation may cause repaints, which in-turn causes jank.
     bool adjustTextAA = !animatingOpacityToScreen && !animatingTransformToScreen;
     // To avoid color fringing, LCD text should only be used on opaque layers with just integral translation.
@@ skipping 25 matching lines @@
         // space which we do not want; so here we use the combinedTransform
         // instead of the drawTransform. However, we do need to add a different
         // scale factor that accounts for the surface's pixel dimensions.
         combinedTransform.Scale(1 / renderSurfaceSublayerScale.x(), 1 / renderSurfaceSublayerScale.y());
         renderSurface->setDrawTransform(combinedTransform);
 
         // The owning layer's transform was re-parented by the surface, so the layer's new drawTransform
         // only needs to scale the layer to surface space.
         layerDrawProperties.target_space_transform.MakeIdentity();
         layerDrawProperties.target_space_transform.Scale(renderSurfaceSublayerScale.x() / layer->contentsScaleX(), renderSurfaceSublayerScale.y() / layer->contentsScaleY());
+        // Eliminate the tiny errors in scales and translations to prevent the errors from propagating,
+        // to prevent bluriness and to ensure LCDText working as expected.
+        roundAlmostIntegerScalesAndTranslationComponents(&layerDrawProperties.target_space_transform);

[enne (OOO), 2013/03/07 19:05:34]

I don't understand what this change does.  The LCD text setting has already been
set above for this layer and the target space transform is not the value that is
propagated down the subtree.

[Xianzhu, 2013/03/07 19:38:17]

Based on the exisiting code, we enable LCD text only when the transform
IsIdentityOrIntegerTranslation, otherwise there might be "color fringing". I
think if this target_spae_transform is not IdentityOrIntegerTranslation, LCD
text may be incorrect (in theory). I don't know much about the underlying
theories :)

[enne (OOO), 2013/03/07 20:19:36]

What you say is correct.  However, we enable LCD text on line 759 and this is
line 796.  Also, the transform that is passed to children is the
combinedTransform (well, later sublayerMatrix), which does not come from
target_space_transform.  So, setting this value will not change LCD text for
this surface's subtree or for this layer.  That's what I'm trying to say.

If you want to affect surfaces, you need to round the combinedTransform up
above, since that's what's used on line 788.

 
         // Inside the surface's subtree, we scale everything to the owning layer's scale.
         // The sublayer matrix transforms layer rects into target
         // surface content space.
         DCHECK(sublayerMatrix.IsIdentity());
         sublayerMatrix.Scale(renderSurfaceSublayerScale.x(), renderSurfaceSublayerScale.y());
 
         // The opacity value is moved from the layer to its surface, so that the entire subtree properly inherits opacity.
         renderSurface->setDrawOpacity(accumulatedDrawOpacity);
         renderSurface->setDrawOpacityIsAnimating(animatingOpacityToTarget);
@@ skipping 437 matching lines @@
 
   // At this point, we think the point does hit the touch event handler region on the layer, but we need to walk up
   // the parents to ensure that the layer was not clipped in such a way that the
   // hit point actually should not hit the layer.
   if (pointIsClippedBySurfaceOrClipRect(screenSpacePoint, layerImpl))
      return false;
 
   return true;
 }
 }  // namespace cc