Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(24)

Issue 1162793003: Add CSS image-orientation: from-image (Closed)

Created:
4 years, 2 months ago by rwlbuis
Modified:
4 years, 1 month ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-paint_chromium.org, blink-reviews-rendering, blink-reviews-style_chromium.org, Rik, danakj, dcheng, dglazkov+blink, dshwang, krit, eae+blinkwatch, f(malita), fs, gyuyoung2, jbroman, jchaffraix+rendering, Justin Novosad, kouhei+svg_chromium.org, leviw+renderwatch, pdr+svgwatchlist_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, Stephen Chennney, slimming-paint-reviews_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Add CSS image-orientation: from-image Support image-orientation: from-image [1] by teaching the CSS style system and image LayoutImage call about it. To keep state about the image-orientation, add a flag on CSS RareNonInheritedData. For now support from-image and the default angle 0deg. Add various tests for computed style of image-orientation as well as its rendering effect on images with EXIF orientation and its dynamic behavior via JS manipulation. Dragging an oriented image works visually, i.e., if the image has a orientation, the drag image should have the same orientation. The behavior partly matches Firefox, in that from-image is supported but not the CSS angle and flip values, unlike Firefox. WebKit has code for using image-orientation CSS with angle values (no from-image or flip), but it is not enabled for Safari. [1] http://www.w3.org/TR/css4-images/#the-image-orientation BUG=158753 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199231

Patch Set 1 #

Patch Set 2 : Try to fix test failures #

Patch Set 3 : Add tests #

Patch Set 4 : Restrict to from-image and 0deg #

Patch Set 5 : Rebase and add test for composited image #

Patch Set 6 : Address compositing issue #

Patch Set 7 : Disable compositing for from-image #

Patch Set 8 : Improved check #

Total comments: 10

Patch Set 9 : Address review comments #

Patch Set 10 : Address even more review comments #

Total comments: 15

Patch Set 11 : More addressing of review comments #

Patch Set 12 : Fix comment #

Patch Set 13 : Add runtime flag #

Total comments: 2

Patch Set 14 : Add runtime_flag dependency #

Patch Set 15 : add converter #

Unified diffs Side-by-side diffs Delta from patch set Stats (+283 lines, -8 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +5 lines, -0 lines 0 comments Download
M LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +35 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +13 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation-dynamic-expected.html View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation-expected.txt View 1 2 3 1 chunk +47 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation-from-image.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +49 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/image-orientation/image-orientation-from-image-composited.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +49 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css/image-orientation/image-orientation-from-image-composited-expected.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
A + LayoutTests/fast/css/image-orientation/image-orientation-from-image-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
A LayoutTests/platform/linux/fast/css/image-orientation/image-orientation-from-image-composited-expected.png View 1 2 3 4 Binary file 0 comments Download
A LayoutTests/platform/linux/fast/css/image-orientation/image-orientation-from-image-expected.png View 1 2 4 Binary file 0 comments Download
M LayoutTests/svg/css/getComputedStyle-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/webexposed/css-properties-as-js-properties-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSComputedStyleDeclaration.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSProperties.in View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSValueKeywords.in View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
M Source/core/css/parser/CSSPropertyParser.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -0 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderConverter.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderConverter.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +7 lines, -0 lines 0 comments Download
M Source/core/frame/UseCounter.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/layout/LayoutImage.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/layout/LayoutImage.cpp View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -0 lines 0 comments Download
M Source/core/layout/LayoutObject.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +16 lines, -4 lines 0 comments Download
M Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +8 lines, -1 line 0 comments Download
M Source/core/style/ComputedStyle.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +5 lines, -0 lines 0 comments Download
M Source/core/style/ComputedStyle.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/style/StyleRareInheritedData.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/style/StyleRareInheritedData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +3 lines, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 53 (7 generated)
rwlbuis
PTAL, but mind you it is WIP:) A lot of it based on WebKit code. ...
4 years, 2 months ago (2015-05-29 22:21:49 UTC) #2
rwlbuis
Next round, I got a bit concerned after seeing this in the spec: ISSUE 7 ...
4 years, 2 months ago (2015-06-03 22:14:45 UTC) #3
pdr.
I'm probably not the best reviewer for this. +cc Noel and Tim, would you be ...
4 years, 2 months ago (2015-06-04 03:53:35 UTC) #5
rwlbuis
On 2015/06/04 03:53:35, pdr wrote: > I'm probably not the best reviewer for this. +cc ...
4 years, 2 months ago (2015-06-04 22:49:27 UTC) #6
Noel Gordon
On 2015/06/04 22:49:27, rwlbuis wrote: > In addition, I was wondering if we could remove ...
4 years, 2 months ago (2015-06-04 23:12:17 UTC) #7
Noel Gordon
Also, I see no changes to CompositedLayerMapping.cpp, which is called CompositedDeprecatedPaintLayerMapping.cpp these days. Please add ...
4 years, 2 months ago (2015-06-04 23:18:13 UTC) #8
rwlbuis
On 2015/06/04 23:18:13, noel gordon wrote: > Also, I see no changes to CompositedLayerMapping.cpp, which ...
4 years, 2 months ago (2015-06-08 15:43:20 UTC) #9
Noel Gordon
On 2015/06/08 15:43:20, rwlbuis wrote: > Thanks for the suggestion, I added a test for ...
4 years, 2 months ago (2015-06-09 11:13:03 UTC) #10
Noel Gordon
On 2015/06/04 23:12:17, noel gordon wrote: > On 2015/06/04 22:49:27, rwlbuis wrote: > > > ...
4 years, 2 months ago (2015-06-09 13:26:55 UTC) #11
rwlbuis
On 2015/06/09 13:26:55, noel gordon wrote: > On 2015/06/04 23:12:17, noel gordon wrote: > > ...
4 years, 2 months ago (2015-06-09 13:47:41 UTC) #12
Noel Gordon
On 2015/06/09 13:47:41, rwlbuis wrote: > On 2015/06/09 13:26:55, noel gordon wrote: > > On ...
4 years, 2 months ago (2015-06-09 14:02:06 UTC) #13
rwlbuis
On 2015/06/09 14:02:06, noel gordon wrote: > > I'll fix the description to something more ...
4 years, 2 months ago (2015-06-09 14:57:29 UTC) #14
Noel Gordon
On 2015/06/09 14:57:29, rwlbuis wrote: > Will have a look. Ok, and because we rename ...
4 years, 2 months ago (2015-06-09 15:17:55 UTC) #15
rwlbuis
On 2015/06/09 15:17:55, noel gordon wrote: > On 2015/06/09 14:57:29, rwlbuis wrote: > > > ...
4 years, 2 months ago (2015-06-09 15:30:57 UTC) #16
Noel Gordon
On 2015/06/09 15:30:57, rwlbuis wrote: > On 2015/06/09 15:17:55, noel gordon wrote: > > On ...
4 years, 2 months ago (2015-06-09 15:38:22 UTC) #17
rwlbuis
On 2015/06/09 15:38:22, noel gordon wrote: > > though when > > used in the ...
4 years, 2 months ago (2015-06-09 16:01:19 UTC) #19
rwlbuis
@noel I am not sure if the TODOs in the Description are the best place ...
4 years, 2 months ago (2015-06-09 21:17:28 UTC) #20
rwlbuis
On 2015/06/09 21:17:28, rwlbuis wrote: > @noel I am not sure if the TODOs in ...
4 years, 2 months ago (2015-06-11 21:32:54 UTC) #21
rwlbuis
On 2015/06/11 21:32:54, rwlbuis wrote: > On 2015/06/09 21:17:28, rwlbuis wrote: > > @noel I ...
4 years, 2 months ago (2015-06-11 21:33:15 UTC) #22
Noel Gordon
On 2015/06/09 21:17:28, rwlbuis wrote: > @noel I am not sure if the TODOs in ...
4 years, 2 months ago (2015-06-12 15:18:38 UTC) #23
Noel Gordon
https://codereview.chromium.org/1162793003/diff/160001/Source/core/layout/LayoutImage.cpp File Source/core/layout/LayoutImage.cpp (right): https://codereview.chromium.org/1162793003/diff/160001/Source/core/layout/LayoutImage.cpp#newcode85 Source/core/layout/LayoutImage.cpp:85: bool hadStyle = (oldStyle != 0); Remove hadStyle, doesn't ...
4 years, 2 months ago (2015-06-12 16:21:05 UTC) #25
Noel Gordon
On 2015/06/12 15:18:38, noel gordon wrote: > I think that angles and flip, force / ...
4 years, 2 months ago (2015-06-12 16:26:53 UTC) #26
rwlbuis
https://codereview.chromium.org/1162793003/diff/160001/Source/core/layout/LayoutImage.cpp File Source/core/layout/LayoutImage.cpp (right): https://codereview.chromium.org/1162793003/diff/160001/Source/core/layout/LayoutImage.cpp#newcode85 Source/core/layout/LayoutImage.cpp:85: bool hadStyle = (oldStyle != 0); On 2015/06/12 16:21:04, ...
4 years, 2 months ago (2015-06-15 17:21:52 UTC) #27
rwlbuis
@noel, PTAL. I think I fixed the Description now. As to the CompositedDeprecatedPaintLayerMapping, I think ...
4 years, 2 months ago (2015-06-18 21:37:09 UTC) #28
Noel Gordon
On 2015/06/18 21:37:09, rwlbuis wrote: > @noel, PTAL. I think I fixed the Description now. ...
4 years, 2 months ago (2015-06-19 07:41:04 UTC) #29
Noel Gordon
https://codereview.chromium.org/1162793003/diff/200001/LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html File LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html (right): https://codereview.chromium.org/1162793003/diff/200001/LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html#newcode6 LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html:6: document.documentElement.className = ""; document.documentElement.className ... not needed I think. ...
4 years, 2 months ago (2015-06-19 07:41:25 UTC) #30
Noel Gordon
ahem: ^^ code might read better if you preferred the early return.
4 years, 2 months ago (2015-06-19 07:43:07 UTC) #31
rwlbuis
https://codereview.chromium.org/1162793003/diff/200001/LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html File LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html (right): https://codereview.chromium.org/1162793003/diff/200001/LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html#newcode6 LayoutTests/fast/css/image-orientation/image-orientation-dynamic.html:6: document.documentElement.className = ""; On 2015/06/19 07:41:24, noel gordon wrote: ...
4 years, 2 months ago (2015-06-19 16:03:16 UTC) #32
Noel Gordon
https://codereview.chromium.org/1162793003/diff/200001/Source/core/layout/LayoutObject.cpp File Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/1162793003/diff/200001/Source/core/layout/LayoutObject.cpp#newcode2296 Source/core/layout/LayoutObject.cpp:2296: // FIXME: crbug.com/498233 On 2015/06/19 16:03:15, rwlbuis wrote: > ...
4 years, 2 months ago (2015-06-19 16:50:07 UTC) #33
rwlbuis
On 2015/06/19 16:50:07, noel gordon wrote: > https://codereview.chromium.org/1162793003/diff/200001/Source/core/layout/LayoutObject.cpp > File Source/core/layout/LayoutObject.cpp (right): > > https://codereview.chromium.org/1162793003/diff/200001/Source/core/layout/LayoutObject.cpp#newcode2296 ...
4 years, 2 months ago (2015-06-19 19:22:09 UTC) #34
Tab Atkins
> Willing to work on that, but fear the patch may get big. I think ...
4 years, 2 months ago (2015-06-22 21:00:38 UTC) #35
rwlbuis
On 2015/06/22 21:00:38, Tab Atkins wrote: > > Willing to work on that, but fear ...
4 years, 2 months ago (2015-06-22 22:02:11 UTC) #36
rwlbuis
@noel ping :)
4 years, 1 month ago (2015-06-26 14:37:46 UTC) #37
rwlbuis
Back from vacation, so another PING :)
4 years, 1 month ago (2015-07-13 14:14:33 UTC) #38
Noel Gordon
Hey (and thanks for setting way status). Perhaps we should add the flag and do ...
4 years, 1 month ago (2015-07-13 14:43:13 UTC) #39
rwlbuis
On 2015/07/13 14:43:13, noel gordon wrote: > Hey (and thanks for setting way status). Perhaps ...
4 years, 1 month ago (2015-07-13 19:58:31 UTC) #40
Timothy Loh
lgtm for css https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in File Source/core/css/CSSProperties.in (right): https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in#newcode198 Source/core/css/CSSProperties.in:198: image-orientation inherited, custom_all Probably better with ...
4 years, 1 month ago (2015-07-16 00:37:47 UTC) #41
Timothy Loh
https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in File Source/core/css/CSSProperties.in (right): https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in#newcode198 Source/core/css/CSSProperties.in:198: image-orientation inherited, custom_all Did you miss runtime_flag=ImageOrientation here?
4 years, 1 month ago (2015-07-16 03:07:26 UTC) #42
rwlbuis
Thanks for the review! On 2015/07/16 03:07:26, Timothy Loh wrote: > https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in > File Source/core/css/CSSProperties.in ...
4 years, 1 month ago (2015-07-16 20:03:31 UTC) #44
Noel Gordon
On 2015/07/16 00:37:47, Timothy Loh wrote: > https://codereview.chromium.org/1162793003/diff/260001/Source/core/css/CSSProperties.in#newcode198 > Source/core/css/CSSProperties.in:198: image-orientation inherited, custom_all > Probably ...
4 years, 1 month ago (2015-07-20 17:07:13 UTC) #45
rwlbuis
On 2015/07/20 17:07:13, noel gordon wrote: > On 2015/07/16 00:37:47, Timothy Loh wrote: > > ...
4 years, 1 month ago (2015-07-20 17:14:41 UTC) #46
Noel Gordon
LGTM
4 years, 1 month ago (2015-07-21 15:17:51 UTC) #47
rwlbuis
On 2015/07/21 15:17:51, noel gordon wrote: > LGTM Thanks! Any PSAs or Intents needed or ...
4 years, 1 month ago (2015-07-21 15:28:40 UTC) #48
Noel Gordon
On 2015/07/21 15:28:40, rwlbuis wrote: > Any PSAs or Intents needed or we do that ...
4 years, 1 month ago (2015-07-21 15:37:10 UTC) #49
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1162793003/320001
4 years, 1 month ago (2015-07-21 15:44:37 UTC) #52
commit-bot: I haz the power
4 years, 1 month ago (2015-07-21 16:59:25 UTC) #53
Message was sent while issue was closed.
Committed patchset #15 (id:320001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199231

Powered by Google App Engine
This is Rietveld 408576698