aboutsummaryrefslogtreecommitdiffhomepage
path: root/tests/CodecAnimTest.cpp
Commit message (Collapse)AuthorAge
* Properly zero in webp if no prior frameGravatar Leon Scroggins III2018-05-24
| | | | | | | | | | | | | | | When erasing the prior frame rect, we need to update the frame rect to take scaling into account. We already do if the prior frame was provided; also do so if it was not provided. Note that this only affects an image with a restore previous frame that is being scaled. webp does not support restore previous, so this will not affect AnimatedImageDrawable in Android. Change-Id: I2d9a4ad45262a0e7afd1134958aff5c6e2ca6687 Reviewed-on: https://skia-review.googlesource.com/115646 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* IWYU for some test files starting with 'C'.Gravatar Ben Wagner2018-03-13
| | | | | | | | | Change-Id: Iea274ae52da8b4b87ec55222c856f40a0d88c4e0 Reviewed-on: https://skia-review.googlesource.com/113746 Reviewed-by: Herb Derby <herb@google.com> Commit-Queue: Ben Wagner <bungeman@google.com> Reviewed-on: https://skia-review.googlesource.com/113944 Reviewed-by: Ben Wagner <bungeman@google.com>
* Revert "IWYU for some test files starting with 'C'."Gravatar Ben Wagner2018-03-12
| | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 1a462bd0fe6fb3cd8d7aa7ad1d8290e764ac79e4. Reason for revert: Broke android. Need to run android bot for these. Original change's description: > IWYU for some test files starting with 'C'. > > Change-Id: I9a9596f7a941cdd8f01e055965c70a4b24438499 > Reviewed-on: https://skia-review.googlesource.com/113746 > Reviewed-by: Herb Derby <herb@google.com> > Commit-Queue: Ben Wagner <bungeman@google.com> TBR=bungeman@google.com,herb@google.com Change-Id: Ic63c2fcc7457e442e9b29a9ccd429927e24e3b77 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/113841 Reviewed-by: Ben Wagner <bungeman@google.com> Commit-Queue: Ben Wagner <bungeman@google.com>
* IWYU for some test files starting with 'C'.Gravatar Ben Wagner2018-03-12
| | | | | | | Change-Id: I9a9596f7a941cdd8f01e055965c70a4b24438499 Reviewed-on: https://skia-review.googlesource.com/113746 Reviewed-by: Herb Derby <herb@google.com> Commit-Queue: Ben Wagner <bungeman@google.com>
* Respect repetition count in SkAnimatedImageGravatar Leon Scroggins III2018-01-22
| | | | | | | | | | | | | | | | | | | | | | | | | | Bug: b/63908092 By default use the repetition count stored in the encoded data (if any). Allow setting the repetition count manually, so that the animation will stop after n+1 total cycles (unless -1 is used for infinite). If the animation is complete, make start reset it. When the animation is not running, make update return max double (i.e. no need to update any time soon). Fix a bug where the first call to update returned -1. Share write_bm with CodecAnimTest, for debugging. Update Sample to check isRunning rather than keeping its own record of whether the animation is running. Change-Id: I883e4d7325f7a7b23a422fa9d756f9ea3018f0f8 Reviewed-on: https://skia-review.googlesource.com/97082 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Use SkAndroidCodec in SkAnimatedImageGravatar Leon Scroggins III2018-01-16
| | | | | | | | | | | | | | | | | | | | | | | | | | Bug: b/63909536 Bug: b/63908092 SkAnimatedImage is designed around a specific Android use case, so move it into the android folders. Make SkAnimatedImage hold an SkAndroidCodec (instead of an SkCodec). Expose fCodec so that SkAnimatedImage can animate by using the internal SkCodec. Update the sample to use SkAndroidCodec. Allow webp to decode a scaled down animation. For RestoreBG frames, adjust the frameRect (which is erased) to account for the scaling. Add a test to verify that we decode a webp with a RestoreBG frame successfully. Disable scaling for later frames in other formats (GIF, for now), since the code for erasing a RestoreBG frame is currently unaware of the sampling. Change-Id: I5dd2b86138f2c7f6adcd08dce1bd49040f7dc224 Reviewed-on: https://skia-review.googlesource.com/94621 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: Derek Sollenberger <djsollen@google.com>
* resources: orgainize directory.Gravatar Hal Canary2017-12-08
| | | | | | | | | Should make it easier to ask just for images. Change-Id: If821743dc924c4bfbc6b2b2d29b14affde7b3afd Reviewed-on: https://skia-review.googlesource.com/82684 Commit-Queue: Hal Canary <halcanary@google.com> Reviewed-by: Leon Scroggins <scroggo@google.com>
* Reland "Hide SkEncodedInfo"Gravatar Leon Scroggins III2017-12-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This partially reverts commit 1793e7bb46c1f9d430c1a699a1c3d3168159b659. Hide SkEncodedInfo Bug: skia:7353 Bug: skia:6839 This contains information that is not necessary for clients to know. The Color enum tells the number of components in the input, but this is only interesting internally (to the SkSwizzler). Similarly, the Alpha enum differs from SkAlphaType in that it has kBinary instead of kPremul. This is useful information only internally for determining whether the SkColorSpaceXform needs to premultiply. The bitsPerComponent is potentially useful for a client; Android (in SkAndroidCodec) uses it to determine the SkColorType. Rather than exposing bitsPerComponent, make SkAndroidCodec a friend so it can access the SkEncodedInfo. A future change will change SkCodec to recommend F16 for bitsPerComponent > 8, but that will be more involved; it was the reason for the revert of this CL. Switch conversionSupported to use an SkColorType, which is enough info. Replace the SkEncodedInfo::Alpha field on SkCodec::FrameInfo with an SkAlphaType. SkCodec still needs an SkEncodedInfo, so move its header (which is already not SK_API) to include/private. TBR=mtklein@chromium.org,reed@google.com Change-Id: I928b1f55317602cb37d29da63b53026c8d139cee Reviewed-on: https://skia-review.googlesource.com/80860 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Revert "Hide SkEncodedInfo"Gravatar Leon Scroggins2017-12-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit c6f7a4ffa9522159efc42f7c948bba5e66bb8844. Reason for revert: Causing differences in Gold, stemming from the fact that this changes the recommended SkImageInfo for 16 bits-per-component PNG from N32 to F16. - an F16 bitmap already png-encodes to a 16 bits-per-component PNG, but it does not encode a linear colorspace (possibly a bug?). when we decode this PNG using getInfo(), it fails because it has an F16 color type and non-linear colorspace. (In the encode-srgb-png gm, this results in blank results for F16.) We could correct this on the encoder side, but it seems possible that a 16 bits-per-component PNG could be encoded with a different color space. In that case, we'd want SkCodec to recommend F16/SRGBLinear, but I think we'd want the SkCodec to store the encoded SkColorSpace so that we can Xform between the two. Currently SkCodec only stores one color space, so that will require a refactor. - When decoding 16-bits-per-component PNGs, we are now decoding them to F16. This shows differences in Gold. The srgb/gpu results now look more like F16. I think this is fine. Original change's description: > Hide SkEncodedInfo > > Bug: skia:7353 > Bug: skia:6839 > > This contains information that is not necessary for clients to know. The > Color enum tells the number of components in the input, but this is only > interesting internally (to the SkSwizzler). > > Similarly, the Alpha enum differs from SkAlphaType in that it has > kBinary instead of kPremul. This is useful information only internally > for determining whether the SkColorSpaceXform needs to premultiply. > > The bitsPerComponent is potentially useful for a client; Android (in > SkAndroidCodec) uses it to determine the SkColorType. Rather than > exposing bitsPerComponent, use it to make the same decision that Android > would have made - 16 bits per component means to set the info to F16. Add > a test that computeOutputColorType behaves as expected. > > Switch conversionSupported to use an SkColorType, which is enough info. > > Replace the SkEncodedInfo::Alpha field on SkCodec::FrameInfo with an > SkAlphaType. > > SkCodec still needs an SkEncodedInfo, so move its header (which is > already not SK_API) to include/private. > > Change-Id: Ie2cf11339bf999ebfd4390c0f448f7edd6feabda > Reviewed-on: https://skia-review.googlesource.com/79260 > Reviewed-by: Mike Reed <reed@google.com> > Reviewed-by: Mike Klein <mtklein@chromium.org> > Commit-Queue: Leon Scroggins <scroggo@google.com> TBR=mtklein@chromium.org,scroggo@google.com,reed@google.com Change-Id: I0c5dd1461e1b70d1e55349a8e7ee6b029c3f556e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:7353, skia:6839 Reviewed-on: https://skia-review.googlesource.com/80660 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Hide SkEncodedInfoGravatar Leon Scroggins III2017-12-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bug: skia:7353 Bug: skia:6839 This contains information that is not necessary for clients to know. The Color enum tells the number of components in the input, but this is only interesting internally (to the SkSwizzler). Similarly, the Alpha enum differs from SkAlphaType in that it has kBinary instead of kPremul. This is useful information only internally for determining whether the SkColorSpaceXform needs to premultiply. The bitsPerComponent is potentially useful for a client; Android (in SkAndroidCodec) uses it to determine the SkColorType. Rather than exposing bitsPerComponent, use it to make the same decision that Android would have made - 16 bits per component means to set the info to F16. Add a test that computeOutputColorType behaves as expected. Switch conversionSupported to use an SkColorType, which is enough info. Replace the SkEncodedInfo::Alpha field on SkCodec::FrameInfo with an SkAlphaType. SkCodec still needs an SkEncodedInfo, so move its header (which is already not SK_API) to include/private. Change-Id: Ie2cf11339bf999ebfd4390c0f448f7edd6feabda Reviewed-on: https://skia-review.googlesource.com/79260 Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Mike Klein <mtklein@chromium.org> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Treat invalid indices as transparent in gifsGravatar Chris Blume2017-09-27
| | | | | | | | | | | | | | | Other browsers (including old Chrome) treat invalid palette indices as transparent for gifs. And there are gifs in the wild which rely on this. As an example, if the palette only has 64 entries (0-63) then index 64 is treated as transparent. BUG=skia:7069 Change-Id: I15e8919a953387506c9ac5945c3ae6a2b90189ab Reviewed-on: https://skia-review.googlesource.com/51100 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Chris Blume <cblume@google.com>
* Call initializeColorXform inside SkCodecGravatar Leon Scroggins III2017-08-18
| | | | | | | | | | | | | | | | | | | | | | | | Make initializeColorXform private, and only call in the base class. Add virtual method to skip initializeColorXform, for classes that do not need one. Change SkCodec::FrameInfo's SkAlphaType to an SkEncodedInfo::Alpha. This allows proper checking internally whether SkCodec needs to do a color correct premultiply. Depends on https://chromium-review.googlesource.com/c/620947, for this API change. (Separated from review.skia.org/25746) Bug: skia:5609 Bug: skia:6839 Change-Id: Icb0d46659c546060c34d32eaf792c86708726c7a Reviewed-on: https://skia-review.googlesource.com/35880 Reviewed-by: Chris Blume <cblume@chromium.org> Reviewed-by: Derek Sollenberger <djsollen@google.com> Reviewed-by: Mike Klein <mtklein@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* use unique_ptr for codec factoriesGravatar Mike Reed2017-07-25
| | | | | | | | | | Will need guards for android (at least) Bug: skia: Change-Id: I2bb8e656997984489ef1f2e41cd3d301c4e7b947 Reviewed-on: https://skia-review.googlesource.com/26040 Reviewed-by: Mike Klein <mtklein@chromium.org> Commit-Queue: Mike Reed <reed@google.com>
* Reland "Remove support for decoding to kIndex_8"Gravatar Leon Scroggins2017-07-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Original change's description: > > Remove support for decoding to kIndex_8 > > > > Fix up callsites, and remove tests that no longer make sense. > > > > Bug: skia:6828 > > Change-Id: I2548c4b7528b7b1be7412563156f27b52c9d4295 > > Reviewed-on: https://skia-review.googlesource.com/21664 > > Reviewed-by: Derek Sollenberger <djsollen@google.com> > > Commit-Queue: Leon Scroggins <scroggo@google.com> > > TBR=djsollen@google.com,scroggo@google.com > > Change-Id: I1bc669441f250690884e75a9a61427fdf75c6907 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: skia:6828 > Reviewed-on: https://skia-review.googlesource.com/22120 > Reviewed-by: Leon Scroggins <scroggo@google.com> > Commit-Queue: Leon Scroggins <scroggo@google.com> TBR=djsollen@google.com,scroggo@google.com Bug: skia:6828 Change-Id: I36ff5a11c529d29e8adc95f43b8edc6fd1dbf5b8 Reviewed-on: https://skia-review.googlesource.com/22320 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Revert "Remove support for decoding to kIndex_8"Gravatar Leon Scroggins2017-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 742a3e298fda669006147e4a305bab8452369b1f. Reason for revert: Breaking Android roll: frameworks/base/core/jni/android/graphics/BitmapFactory.cpp:453:18: error: no member named 'fColorPtr' in 'SkAndroidCodec::AndroidOptions' codecOptions.fColorPtr = colorPtr; ~~~~~~~~~~~~ ^ frameworks/base/core/jni/android/graphics/BitmapFactory.cpp:454:18: error: no member named 'fColorCount' in 'SkAndroidCodec::AndroidOptions' codecOptions.fColorCount = colorCount; ~~~~~~~~~~~~ ^ Original change's description: > Remove support for decoding to kIndex_8 > > Fix up callsites, and remove tests that no longer make sense. > > Bug: skia:6828 > Change-Id: I2548c4b7528b7b1be7412563156f27b52c9d4295 > Reviewed-on: https://skia-review.googlesource.com/21664 > Reviewed-by: Derek Sollenberger <djsollen@google.com> > Commit-Queue: Leon Scroggins <scroggo@google.com> TBR=djsollen@google.com,scroggo@google.com Change-Id: I1bc669441f250690884e75a9a61427fdf75c6907 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:6828 Reviewed-on: https://skia-review.googlesource.com/22120 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Remove support for decoding to kIndex_8Gravatar Leon Scroggins III2017-07-10
| | | | | | | | | | Fix up callsites, and remove tests that no longer make sense. Bug: skia:6828 Change-Id: I2548c4b7528b7b1be7412563156f27b52c9d4295 Reviewed-on: https://skia-review.googlesource.com/21664 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Make SkCodec more flexible about its required frameGravatar Leon Scroggins III2017-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | SkCodec sets fRequiredFrame to be the earliest possible frame that a given frame can depend on. e.g. - Frame A fills the screen, Keep - Frame B does not cover A, Keep - Frame C covers B but not A, and is opaque Frame C can depend on either A or B. SkCodec already reports that C depends on A. This CL allows a client of SkCodec to use either A or B to create C. Also expose the DisposalMethod. Since any frame between A and C can be used to create C except for DisposePrevious frames, the client needs to be able to know the disposal method so they do not try to use such a frame to create C. Further, the disposal method can be used to give the client a better idea whether they will continue to need a frame. (e.g. if frame i is DisposePrevious and depends on i-1, the client may not want to steal i-1 to create i, since i+1 may also depend on i-1.) TODO: Share code for decoding prior frames between GIF and WEBP Change-Id: I91a5ae22ba3d8dfbe0bde833fa67ae3da0d81ed6 Reviewed-on: https://skia-review.googlesource.com/13722 Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Chris Blume <cblume@chromium.org> Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Fix alpha issues in SkGifCodecGravatar Leon Scroggins III2017-06-07
| | | | | | | | | | | | | | | | - Call conversion_possible with the proper alpha type for the frame. - Always use kUnpremul for the transform. Previously we used the alpha type for the first frame. If it was opaque and a later frame was not, this would be incorrect. Also fix Codec_frames test. Most of the tests were not running due to a return statement in a loop. Change that to continue, and correct errors in the test. Provide better debugging information. Change-Id: Icd40c09526b1d599168bfe90d93d8ddcdd9ca20f Reviewed-on: https://skia-review.googlesource.com/18935 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Add animation support to SkWebpCodecGravatar Leon Scroggins III2017-05-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | TBR=reed@google.com (No change to the public API, but changed a header file) SkWebpCodec: - Implement onGetFrameCount, onGetFrameInfo, and onGetRepetitionCount - Respect the alpha reported by libwebp. Although the spec states that it is only a hint, the libwebp encoder uses it properly. Respecting allows us to draw opaque images faster and decode them to 565. This also matches other SkCodecs (and Chromium). - onGetPixels: - Decode the frame requested, recursively decoding required frame if necessary - When blending with a prior frame, use SkRasterPipeline SkCodec: - Move check for negative index to getFrameInfo - Reset the colorXform if one is not needed SkCodecAnimation: - Add new blend enum, for WebP's (and APNG's) non-blending option SkFrameHolder: - New base classes for frames and the owner of the frames, allowing code sharing between SkWebpCodec and SkGifCodec (particularly for determining whether a frame has alpha and what frame it depends on) - When moving items from SkGIFFrameContext, use Skia conventions (i.e. int instead of unsigned) - Rename "delay time" to "duration", to match e.g. SkFrameInfo:: fDuration SkGifImageReader: - Move pieces to SkFrameHolder, and adapt to changes made in the process - Make setAlphaAndRequiredFrame (now on the base class SkFrameHolder) more general to support webp, and add support for frames that do not blend - Change SkGIFFrameContext from a struct to a class, to match how we use the distinction elsewhere (i.e. struct is a small object with public fields) - Rework hasTransparentPixel (now hasTransparency, since it returns true in some cases where there is not a transparent pixel) to better fit with the modified setAlphaAndRequiredFrame. Also be more consistent when there is no transparent pixel but no color map. - Simplify an if condition that was previously simplified in 2d61e717 but accidentally got reverted in a4db9be6 CodecAnimTest: - Test new animated webp files - Rearrange the test to more cleanly print alpha type mismatches for the first frame resources: - webp-animated.webp - animated webp from Chromium - blendBG.webp - new webp file using bits of webp-animated-semitransparent4.webp from Chromium - tests required frame and alpha when using the non-blending mode - frames have the following properties: - Frame 0: no alpha, fills screen - Frame 1: alpha, fills screen - Frame 2: no alpha, fills screen - Frame 3: alpha, fills screen, blendBG - Frame 4: no alpha, fills screen, blendBG - Frame 5: alpha, blendBG - Frame 6: covers 4, has alpha, blendBG - also used to test decoding to 565 if the new frame data has alpha but blends onto an opaque frame DM.cpp: - Test animated images to non-native 8888 and unpremul DMSrcSink.cpp: - Do not test non-native 8888 decodes to f16 dst - Test unpremul decodes to f16 - Copy a frame of an animated image prior to drawing, since in unpremul mode, the DM code will premultiply first. Bug: skia: 3315 Change-Id: I4e55ae2ee5bc095b37a743bdcfac644be603b980 Reviewed-on: https://skia-review.googlesource.com/16707 Commit-Queue: Mike Reed <reed@google.com> Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Leon Scroggins <scroggo@google.com> Reviewed-by: Matt Sarett <msarett@google.com>
* Revert "Revert "Delete copyTo(Allocator), hide copyTo() behind flag""Gravatar Matt Sarett2017-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 0122af08f6af0dee490e1a4f35b552377d0d4753. Reason for revert: Fixed Android callsite Original change's description: > Revert "Delete copyTo(Allocator), hide copyTo() behind flag" > > This reverts commit d4a338f4d0a0cdc08d7d3668931c60997f0fa971. > > Reason for revert: Looks like I missed something I was supposed to delete in Android. > > Original change's description: > > Delete copyTo(Allocator), hide copyTo() behind flag > > > > Replace uses of copyTo() in Skia. > > > > Bug: skia:6464 > > Change-Id: I921dc53a1c29a5176d18f05741f7c0b5a008e548 > > Reviewed-on: https://skia-review.googlesource.com/14502 > > Commit-Queue: Matt Sarett <msarett@google.com> > > Reviewed-by: Mike Reed <reed@google.com> > > > > TBR=msarett@google.com,reed@google.com > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > > Change-Id: I4d252940cc6a2462b030007055ea6c229471fc6e > Reviewed-on: https://skia-review.googlesource.com/14602 > Reviewed-by: Matt Sarett <msarett@google.com> > Commit-Queue: Matt Sarett <msarett@google.com> > TBR=msarett@google.com,reviews@skia.org,reed@google.com Change-Id: I81659a820f79f1958fda23cb62513065b57db99d Reviewed-on: https://skia-review.googlesource.com/14640 Commit-Queue: Matt Sarett <msarett@google.com> Reviewed-by: Matt Sarett <msarett@google.com>
* Revert "Delete copyTo(Allocator), hide copyTo() behind flag"Gravatar Matt Sarett2017-04-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit d4a338f4d0a0cdc08d7d3668931c60997f0fa971. Reason for revert: Looks like I missed something I was supposed to delete in Android. Original change's description: > Delete copyTo(Allocator), hide copyTo() behind flag > > Replace uses of copyTo() in Skia. > > Bug: skia:6464 > Change-Id: I921dc53a1c29a5176d18f05741f7c0b5a008e548 > Reviewed-on: https://skia-review.googlesource.com/14502 > Commit-Queue: Matt Sarett <msarett@google.com> > Reviewed-by: Mike Reed <reed@google.com> > TBR=msarett@google.com,reed@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I4d252940cc6a2462b030007055ea6c229471fc6e Reviewed-on: https://skia-review.googlesource.com/14602 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Matt Sarett <msarett@google.com>
* Delete copyTo(Allocator), hide copyTo() behind flagGravatar Matt Sarett2017-04-27
| | | | | | | | | | Replace uses of copyTo() in Skia. Bug: skia:6464 Change-Id: I921dc53a1c29a5176d18f05741f7c0b5a008e548 Reviewed-on: https://skia-review.googlesource.com/14502 Commit-Queue: Matt Sarett <msarett@google.com> Reviewed-by: Mike Reed <reed@google.com>
* Switch SkCodec to int for counts and indicesGravatar Leon Scroggins III2017-04-17
| | | | | | | | | | | | This matches other Skia APIs. size_t was adopted from blink/ GIFImageReader. Change-Id: Ic83e59f0942f597c4fb834e623acd9886ad483fe Reviewed-on: https://skia-review.googlesource.com/13274 Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Matt Sarett <msarett@google.com> Reviewed-by: Chris Blume <cblume@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Add a test for getFrameInfo on truncated dataGravatar Leon Scroggins III2017-04-17
| | | | | | | | | | | | | | Prior to e132e7be5f9108692254c37db592ea7611abbc15, calling getFrameInfo on an SkGifCodec which was truncated before fully parsing the local color map of the first frame would hit an assert. Add a test to verify that the problem has been fixed. Modify box.gif to use a local color map for the first (and only) frame. Change-Id: I905afe60c0c6ecc82e251e00ef82b2e5b22975a3 Reviewed-on: https://skia-review.googlesource.com/13320 Reviewed-by: Chris Blume <cblume@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Add SkCodec methods for individual framesGravatar Leon Scroggins III2017-04-12
| | | | | | | | | | | | | | | | | | | | | Add a version of getFrameInfo that returns information about a single frame, allowing a client to skip creating the entire vector. Add getFrameCount, for determining the number of frames in the image. Reimplement std::vector<FrameInfo> getFrameInfo with the new methods. Updates to the test: - getFrameInfo(size_t, FrameInfo*) fails before parsing - Test both versions of getFrameInfo - Recreate the codec between tests, to test parsing Change-Id: I77c19087f2f8dcf2c536d80167b18ad1ca96ae94 Reviewed-on: https://skia-review.googlesource.com/13190 Reviewed-by: Matt Sarett <msarett@google.com> Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Chris Blume <cblume@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Correct GIF frame dependencies and track alphaGravatar Leon Scroggins III2017-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add SkCodec::FrameInfo::fAlphaType. The SkImageInfo for the SkCodec specifies the SkAlphaType for the first frame, but the opacity can vary from frame to frame. When determining the required frame, also compute whether a frame has alpha. Update how we determine the required frame, which had bugs. (Update a test that had an incorrect required frame as a result.) Add new test images covering cases that have been fixed: - randPixelsAnim2.gif It has the following frames: A (keep) B (keep) (subset) C (disposePrevious) (covers B) D (any) (does *not* cover B) B and C depend on A, but D depends on B, since after disposing C, B should be visible again. - alphabetAnim.gif Includes frames which fill the image size, with different disposal methods and transparencies. Change-Id: Ie086167711c4cac4931ed8c4ddaeb9c9b0b91fdb Reviewed-on: https://skia-review.googlesource.com/9810 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: Mike Reed <reed@google.com> Reviewed-by: Matt Sarett <msarett@google.com>
* GIF: Better check for frame dependencyGravatar Leon Scroggins III2017-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a frame does not have a valid transparent index and it covers the prior frame, it does not really depend on that frame. Instead, it depends on the frame that the prior frame depends on. Determine this once we have parsed the local color map (if any), so a transparent index out of range of the color map is not considered valid. Share code that determines whether a frame has a transparent pixel. Add a test that we compute the dependencies correctly. randPixelsAnim.gif has 13 frames. After the first, the frames cover all combinations of - Whether the prior frame was keep, restoreBG or restoreToPrevious - Whether the new frame covers the prior frame - Whether the new frame has a transparent pixel (It only does so when using a global color table. It may make sense to expand the test to also cover using local color tables.) The test caught a bug where we incorrectly reused an existing SkColorTable for a different frame. Fix that bug by keeping track of the transparent index associated with the current SkColorTable. Change-Id: I3cf6be7f612990fa7a00d9e74d116d31bd227526 Reviewed-on: https://skia-review.googlesource.com/6402 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Report repetition count in SkCodecGravatar scroggo2016-11-01
| | | | | | | | | | | | | | | | | | | | | Add a new accessor to retrieve the repetition count. Remove constants (and corresponding copyright) in SkCodecAnimation. These may make sense for the calling code, but are not needed here. kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite. Move cLoopCountNotSeen to private. It is used to determine whether we still need to parse. Add a new enum to the parse query - only parse enough to determine the repetition count. Unlike Chromium, SkGifCodec does not account for deleting the reader (which SkGifCodec does not do) or failed decodes. Add a test. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002 Review-Url: https://codereview.chromium.org/2447863002
* Use correct color table for prior GIF framesGravatar Leon Scroggins III2016-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | While investigating skbug.com/5883 I noticed that we use the color table for the current frame even while recursively decoding frames that the current frame depends on. This CL updates fCurrColorTable, fCurrColorTableIsReal and fSwizzler before decoding prior frames, and then sets them back afterwards. Move telling the client about the color table into prepareToDecode, since the other callers do not need to do so. (That is only necessary for decoding to index 8, which is unsupported for frames with dependencies.) Add a test that exposes the bug. colorTables.gif has a local color table in its second frame that does not match the global table used by the first frame. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4208 Change-Id: Id2dc9e3283adfd92801d2f38726afa74574b1955 Reviewed-on: https://skia-review.googlesource.com/4208 Reviewed-by: Joost Ouwerling <joostouwerling@google.com> Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Include "SkBitmap.h" in CodecAnimTest.cppGravatar scroggo2016-10-24
| | | | | | | Fixes build GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2449513002 Review-Url: https://codereview.chromium.org/2449513002
* Add support for multiple frames in SkCodecGravatar scroggo2016-10-24
Add an interface to decode frames beyond the first in SkCodec, and add an implementation for SkGifCodec. Add getFrameData to SkCodec. This method reads ahead in the stream to return a vector containing meta data about each frame in the image. This is not required in order to decode frames beyond the first, but it allows a client to learn extra information: - how long the frame should be displayed - whether a frame should be blended with a prior frame, allowing the client to provide the prior frame to speed up decoding Add a new fields to SkCodec::Options: - fFrameIndex - fHasPriorFrame The API is designed so that SkCodec never caches frames. If a client wants a frame beyond the first, they specify the frame in Options.fFrameIndex. If the client does not have the frame's required frame (the frame that this frame must be blended on top of) cached, they pass false for Options.fHasPriorFrame. Unless the frame is independent, the codec will then recursively decode all frames necessary to decode fFrameIndex. If the client has the required frame cached, they can put it in the dst they pass to the codec, and the codec will only draw fFrameIndex onto it. Replace SkGifCodec's scanline decoding support with progressive decoding, and update the tests accordingly. Implement new APIs in SkGifCodec. Instead of using gif_lib, use GIFImageReader, imported from Chromium (along with its copyright headers) with the following changes: - SkGifCodec is now the client - Replace blink types - Combine GIFColorMap::buildTable and ::getTable into a method that creates and returns an SkColorTable - Input comes from an SkStream, instead of a SegmentReader. Add SkStreamBuffer, which buffers the (potentially partial) stream in order to decode progressively. (FIXME: This requires copying data that previously was read directly from the SegmentReader. Does this hurt performance? If so, can we fix it?) - Remove UMA code - Instead of reporting screen width and height to the client, allow the client to query for it - Fail earlier if the first frame AND screen have size of zero - Compute required previous frame when adding a new one - Move GIFParseQuery from GIFImageDecoder to GIFImageReader - Allow parsing up to a specific frame (to skip parsing the rest of the stream if a client only wants the first frame) - Compute whether the first frame has alpha and supports index 8, to create the SkImageInfo. This happens before reporting that the size has been decoded. Add GIFImageDecoder::haveDecodedRow to SkGifCodec, imported from Chromium (along with its copyright header), with the following changes: - Add support for sampling - Use the swizzler - Keep track of the rows decoded - Do *not* keep track of whether we've seen alpha Remove SkCodec::kOutOfOrder_SkScanlineOrder, which was only used by GIF scanline decoding. Call onRewind even if there is no stream (SkGifCodec needs to clear its decoded state so it will decode from the beginning). Add a method to SkSwizzler to access the offset into the dst, taking subsetting into account. Add a GM that animates a GIF. Add tests for the new APIs. *** Behavior changes: * Previously, we reported that an image with a subset frame and no transparent index was opaque and used the background index (if present) to fill the background. This is necessary in order to support index 8, but it does not match viewers/browsers I have seen. Examples: - Chromium and Gimp render the background transparent - Firefox, Safari, Linux Image Viewer, Safari Preview clip to the frame (for a single frame image) This CL matches Chromium's behavior and renders the background transparent. This allows us to have consistent behavior across products and simplifies the code (relative to what we would have to do to continue the old behavior on Android). It also means that we will no longer support index 8 for some GIFs. * Stop checking for GIFSTAMP - all GIFs should be either 89a or 87a. This matches Chromium. I suspect that bugs would have been reported if valid GIFs started with "GIFVER" instead of "GIF89a" or "GIF87a" (but did not decode in Chromium). *** Future work not included in this CL: * Move some checks out of haveDecodedRow, since they are the same for the entire frame e.g. - intersecting the frameRect with the full image size - whether there is a color table * Change when we write transparent pixels - In some cases, Chromium deemed this unnecessary, but I suspect it is slower than the fallback case. There will continue to be cases where we should *not* write them, but for e.g. the first pass where we have already cleared to transparent (which we may also be able to skip) writing the transparent pixels will not make anything incorrect. * Report color type and alpha type per frame - Depending on alpha values, disposal methods, frame rects, etc, subsequent frames may have different properties than the first. * Skip copies of the encoded data - We copy the encoded data in case the stream is one that cannot be rewound, so we can parse and then decode (possibly not immediately). For some input streams, this is unnecessary. - I was concerned this cause a performance regression, but on average the new code is faster than the old for the images I tested [1]. - It may cause a performance regression for Chromium, though, where we can always move back in the stream, so this should be addressed. Design doc: https://docs.google.com/a/google.com/document/d/12Qhf9T92MWfdWujQwCIjhCO3sw6pTJB5pJBwDM1T7Kc/ [1] https://docs.google.com/a/google.com/spreadsheets/d/19V-t9BfbFw5eiwBTKA1qOBkZbchjlTC5EIz6HFy-6RI/ GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=2045293002 Review-Url: https://codereview.chromium.org/2045293002
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-22 15:45:17 +0000