Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(285)

Issue 68203023: Merge FixedIntegerConversion and FixedFloatConversion for CSSPrimitiveValue::convertToLength (Closed)

Created:
7 years, 1 month ago by Timothy Loh
Modified:
7 years, 1 month ago
CC:
blink-reviews, dglazkov+blink, apavlov+blink_chromium.org, darktears
Base URL:
https://chromium.googlesource.com/chromium/blink@master
Visibility:
Public.

Description

Merge FixedIntegerConversion and FixedFloatConversion for CSSPrimitiveValue::convertToLength Continuing from crrev.com/71253002, the two Fixed conversion options in convertToLength behave almost identically, save for some clamping differences. The FixedIntegerConversion clamps to about the range of LayoutUnits (+-32mil) and doesn't actually convert to an integer. FixedFloatConversion simple doesn't clamp. Almost all Length conversions use the clamped path and it doesn't seem like the differences are intentional. The non-clamping paths are calcs, blurs in filters, and lengths in transforms. I've made all the conversions clamp for consistency in getComputedStyle (as otherwise properties which return used values will be clamped and those which return computed values won't). For reference the clamping behaviour was introduced in wkb.ug/102735 BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162038

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -26 lines) Patch
M Source/core/css/BasicShapeFunctions.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSCalculationValue.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSPrimitiveValueMappings.h View 1 chunk +7 lines, -10 lines 0 comments Download
M Source/core/css/resolver/FilterOperationResolver.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.cpp View 10 chunks +12 lines, -12 lines 0 comments Download
M Source/core/css/resolver/TransformBuilder.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
Timothy Loh
7 years, 1 month ago (2013-11-14 06:38:43 UTC) #1
eae
LGTM
7 years, 1 month ago (2013-11-14 16:37:06 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/timloh@chromium.org/68203023/1
7 years, 1 month ago (2013-11-14 16:37:18 UTC) #3
commit-bot: I haz the power
7 years, 1 month ago (2013-11-14 18:33:05 UTC) #4
Message was sent while issue was closed.
Change committed as 162038

Powered by Google App Engine
This is Rietveld 408576698