Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2100483003:
cc: Change directly composited image raster source scale occasionally. (Closed)
Descriptioncc: Change directly composited image raster source scale occasionally.
This patch changes the behavior of directly composited images in the
following way:
- The scale starts at 1.f.
- It is halved if the ideal source scale is less than half of the raster
scale.
- It is doubled if the ideal source scale is greater than the raster
scale.
R=enne, chrishtr, danakj
BUG=606287
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/c05804da22bca99f807b9198465dbe1318dbfa76
Cr-Commit-Position: refs/heads/master@{#402333}
Patch Set 1 #Patch Set 2 : remove comment #Patch Set 3 : update #Patch Set 4 : fix #Patch Set 5 : tests #Messages
Total messages: 21 (8 generated)
Description was changed from ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr BUG=606287 ========== to ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Please take a look. This heuristic seems to work OK in the sense that an animated/scaling image adjusts its raster scale once in a while, and it uses a lot less memory (see example in the referenced bug). I'm happy to change the heuristic if we can come up with a better one. One issue with this heuristic is that we want to adjust the scale as soon as the ideal is larger, which means we can run into the following situation: ideal 0.49, raster 0.25 ideal 0.50, raster 0.50 ideal 0.51, raster 1.00 ideal 0.50, raster 1.00 ideal 0.49, raster 0.50 ideal 0.50, raster 0.50 ideal 0.51, raster 1.00 We can maybe adjust it in the down direction by 1/2 if it differs by more than 4x, then we'd end up with: ideal 0.49, raster 0.25 ideal 0.50, raster 0.50 ideal 0.51, raster 1.00 ideal 0.50, raster 1.00 ideal 0.49, raster 1.00 ideal 0.50, raster 1.00 ideal 0.51, raster 1.00 which seems more stable. Any other suggestions are more than welcome!
Description was changed from ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr, danakj BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
vmpstr@chromium.org changed reviewers: + danakj@chromium.org
+danakj as well
It's a little sad how disconnected directly composited images are from the rest of the image decoding pipeline. Maybe we can fix this at some point in the far slimming paint future. I feel like images behave better under scale than a lot of other content, so I'd prefer to avoid redecoding churn when we can and be more stable. Maybe you could give each scale some sort of overlapping range where it's valid to achieve more hysteresis? e.g. scale x is valid from 0.5*x to 1.5*x, and don't change raster scale unless the ideal moves out of that range? I'm not super concerned about scaling up and making that look more crisp, since I feel like I've never heard a complaint on that front, but maybe that's a change we should make to be more consistent with what we do with rerastering content under scale changes.
On 2016/06/24 22:38:51, enne wrote: > It's a little sad how disconnected directly composited images are from the rest > of the image decoding pipeline. Maybe we can fix this at some point in the far > slimming paint future. > > I feel like images behave better under scale than a lot of other content, so I'd > prefer to avoid redecoding churn when we can and be more stable. Maybe you > could give each scale some sort of overlapping range where it's valid to achieve > more hysteresis? e.g. scale x is valid from 0.5*x to 1.5*x, and don't change > raster scale unless the ideal moves out of that range? > > I'm not super concerned about scaling up and making that look more crisp, since > I feel like I've never heard a complaint on that front, but maybe that's a > change we should make to be more consistent with what we do with rerastering > content under scale changes. I'm a little concerned about a page that scales the image right to the upper bound of the range and leaves it like that. That image would be blurry indefinitely. I've changed this so that we double it when it is outside of x4 range, I think this makes more sense. The above example I had is a bit off, because it should never be the case that ideal = 0.49 and raster is 0.25.. The actual example is: ideal 0.25, raster 0.25 ideal 0.26, raster 0.50 ... ideal 0.49, raster 0.50 ideal 0.50, raster 0.50 ideal 0.51, raster 1.00 ideal 0.50, raster 1.00 ideal 0.49, raster 1.00 ideal 0.50, raster 1.00 ideal 0.51, raster 1.00 ideal 0.50, raster 1.00 ... ideal 0.25, raster 1.00 ideal 0.24, raster 0.50 I think that's pretty stable.
Ok, that sounds reasonable, especially re: trying to avoid being blurry indefinitely.
Could you maybe write a unit test to just make sure this logic is doing the right thing? I know we have some others for other kinds of scale changes.
Added a test, PTAL.
lgtm, thanks!
The CQ bit was checked by vmpstr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by vmpstr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr, danakj BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr, danakj BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr, danakj BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: Change directly composited image raster source scale occasionally. This patch changes the behavior of directly composited images in the following way: - The scale starts at 1.f. - It is halved if the ideal source scale is less than half of the raster scale. - It is doubled if the ideal source scale is greater than the raster scale. R=enne, chrishtr, danakj BUG=606287 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/c05804da22bca99f807b9198465dbe1318dbfa76 Cr-Commit-Position: refs/heads/master@{#402333} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/c05804da22bca99f807b9198465dbe1318dbfa76 Cr-Commit-Position: refs/heads/master@{#402333} |
