Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
733583003:
Don't turn off anti-aliasing for image backgrounds unless the device scale factor is large enough. (Closed)
|
Created:
6 years, 1 month ago by chrishtr Modified:
6 years, 1 month ago Reviewers:
enne (OOO) CC:
blink-reviews, blink-reviews-paint_chromium.org, slimming-paint-reviews_chromium.org Base URL:
svn://svn.chromium.org/blink/trunk Project:
blink Visibility:
Public. |
DescriptionDon't turn off anti-aliasing for image backgrounds unless the device scale factor is
large enough.
Otherwise, rasterized output of cases such as sprited backgrounds of <img> tags will
have poor quality.
BUG=423834, 433282
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185461
Patch Set 1 #
Total comments: 3
Patch Set 2 : #
Total comments: 1
Patch Set 3 : #Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #
Messages
Total messages: 32 (10 generated)
chrishtr@chromium.org changed reviewers: + enne@chromium.org
https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... File Source/core/paint/ImagePainter.cpp (right): https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... Source/core/paint/ImagePainter.cpp:203: float deviceScaleFactor(LocalFrame*); drive-by: Why not include Page.h? Declaring extern functions within source files seems odd.
http://i.imgur.com/7Uj3bBL.gif https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... File Source/core/paint/ImagePainter.cpp (right): https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... Source/core/paint/ImagePainter.cpp:203: float deviceScaleFactor(LocalFrame*); What is this, an extern? Maybe you should just add the right include?
https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... File Source/core/paint/ImagePainter.cpp (right): https://codereview.chromium.org/733583003/diff/1/Source/core/paint/ImagePaint... Source/core/paint/ImagePainter.cpp:203: float deviceScaleFactor(LocalFrame*); On 2014/11/14 19:13:47, jbroman wrote: > drive-by: Why not include Page.h? Declaring extern functions within source files > seems odd. Done.
https://codereview.chromium.org/733583003/diff/20001/Source/core/rendering/Re... File Source/core/rendering/RenderImage.cpp (right): https://codereview.chromium.org/733583003/diff/20001/Source/core/rendering/Re... Source/core/rendering/RenderImage.cpp:41: #include "core/page/Page.h" Can you include this in ImagePainter?
On 2014/11/14 at 19:17:53, enne wrote: > https://codereview.chromium.org/733583003/diff/20001/Source/core/rendering/Re... > File Source/core/rendering/RenderImage.cpp (right): > > https://codereview.chromium.org/733583003/diff/20001/Source/core/rendering/Re... > Source/core/rendering/RenderImage.cpp:41: #include "core/page/Page.h" > Can you include this in ImagePainter? Done.
PS please wrap your change description at 72 characters. This is git, not Blink. ;)
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733583003/20001
The CQ bit was unchecked by enne@chromium.org
Can you upload the patch that did what you said you did?
On 2014/11/14 at 21:25:43, enne wrote: > Can you upload the patch that did what you said you did? Done. ImagePainter was already including Page.h
LGTM
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733583003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733583003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/36646)
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733583003/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/31025)
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733583003/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as 185461 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
