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

Issue 1277593002: Support decoding PNG to 565. (Closed)

Created:
5 years, 4 months ago by scroggo_chromium
Modified:
5 years, 4 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Support decoding PNG to 565. Although we initially discussed not supporting 565, SkCodec needs to support Android's BitmapFactory and BitmapRegionDecoder, which need to support 565. We could instead implement 565 on top of SkCodec, but this would require more memory and run more slowly. An open question is whether to support dithering, and how. In order to support dithering, we need to pass y to RowProc, which I believe means we will need to pass y to SkSwizzler::swizzle(). I dislike having an option which is typically ignored - SkImageDecoder allows you to turn on dithering when decoding to any color type, but it is only meaningful only if the output color type is 565. We could also make the client implement dithering, which I believe would mean forcing them to do the conversion to 565 (since we have already thrown away data when we convered to 565 without dithering). BUG=skia:3257 Committed: https://skia.googlesource.com/skia/+/ab60c5bd9f360e9d58f0f34683b92fb841a514b1

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -1 line) Patch
M src/codec/SkCodec_libpng.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M src/codec/SkSwizzler.cpp View 7 chunks +50 lines, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
scroggo_chromium
5 years, 4 months ago (2015-08-05 20:20:34 UTC) #2
msarett
lgtm
5 years, 4 months ago (2015-08-06 12:20:08 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1277593002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1277593002/1
5 years, 4 months ago (2015-08-06 13:01:15 UTC) #5
commit-bot: I haz the power
5 years, 4 months ago (2015-08-06 13:08:21 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/ab60c5bd9f360e9d58f0f34683b92fb841a514b1

Powered by Google App Engine
This is Rietveld 408576698