aboutsummaryrefslogtreecommitdiffhomepage
path: root/third_party/gif
Commit message (Collapse)AuthorAge
* Move setAlphaAndRequiredFrame to src/codecGravatar Nigel Tao2018-07-30
| | | | | | | | | | | | | The corresponding declaration is already in src/codec/SkFrameHolder.h That code is not solely used by third_party/gif, and in the future, we may have another GIF implementation. Bug: skia: Change-Id: If04cfd4a5b1106f380c89e11c34a8d854ce2b05a Reviewed-on: https://skia-review.googlesource.com/144240 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Remove unused field/methodsGravatar Leon Scroggins III2018-01-25
| | | | | | | | | These have been replaced by duration on the base class. Change-Id: Id87ecf09f4da6840280e3f5657453915a8d3c81b Reviewed-on: https://skia-review.googlesource.com/99245 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: 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>
* Remove unused param to SkGIFFrameContext ctorGravatar Chris Blume2017-09-28
| | | | | | | | | | | | | After updating the transparency index code, the reader parameter to SkGIFFrameContext's ctor is no longer needed. This patch removes it. BUG=skia:7069 Change-Id: If129f825639e8d43d73794adca2de09785f56a3c Reviewed-on: https://skia-review.googlesource.com/52602 Commit-Queue: Chris Blume <cblume@chromium.org> Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: 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>
* GIF: Use SkTArray instead of std::vectorGravatar Leon Scroggins III2017-08-30
| | | | | | | | | | | | | Speculative fix for a memory regression seen in Chromium. Chromium previously used a WTF::Vector, which has a growth factor of 1.5, as does SkTArray. Depending on the implementation of std::vector, this may slow the allocation of memory. Bug: 758946 Change-Id: I323390027467e32a6c66667c927fae0aba292446 Reviewed-on: https://skia-review.googlesource.com/40777 Reviewed-by: Ben Wagner <bungeman@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Convert NULL and 0 to nullptr.Gravatar Ben Wagner2017-08-28
| | | | | | | | | | | | | | This was created by looking at warnings produced by clang's -Wzero-as-null-pointer-constant. This updates most issues in Skia code. However, there are places where GL and Vulkan want pointer values which are explicitly 0, external headers which use NULL directly, and possibly more uses in un-compiled sources (for other platforms). Change-Id: Id22fbac04d5c53497a53d734f0896b4f06fe8345 Reviewed-on: https://skia-review.googlesource.com/39521 Reviewed-by: Mike Reed <reed@google.com> Commit-Queue: Ben Wagner <bungeman@google.com>
* Make haveDecodedRow return voidGravatar Leon Scroggins III2017-08-22
| | | | | | | | | | The method already always returns true, except in a single case after asserting. Change-Id: Icf241a8af04220d459c0782ffd9b74c34c753236 Reviewed-on: https://skia-review.googlesource.com/37161 Reviewed-by: Chris Blume <cblume@chromium.org> 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>
* Report first GIF frame after knowing its meta dataGravatar Leon Scroggins III2017-07-19
| | | | | | | | | | | | | | | | | Previously, we reported the first image as soon as it was available. As a result, in crrev.com/2565323003, InitializeNewFrame might be called before the metadata is known, meaning it would read the wrong metadata. Instead of looking at the imagesCount(), SkGifCodec::NewFromStream looks at frameContext(0), which may still exist even if it's not yet counted in imagesCount(). Add a test that confirms the desired behavior. Change-Id: Ib392721ecd2218ba0fcd35aaa64117c0ba3e4ea6 Reviewed-on: https://skia-review.googlesource.com/24405 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Report error on failure to create SkCodecGravatar Leon Scroggins III2017-07-14
| | | | | | | | | | | | | | | | | | Update NewFromStream to report an error on failure to create an SkCodec, so that a client can distinguish between - not enough data - invalid data In Chromium, this will allow blink::ImageDecoder to call SetFailed if the stream is invalid early and we never create an SkCodec. Without this, ImageDecoder will keep trying to create an SkCodec when it receives more data. Change-Id: I4f505c56d91c982be36a828fd0f7db17b1596588 Reviewed-on: https://skia-review.googlesource.com/22642 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: Derek Sollenberger <djsollen@google.com> Reviewed-by: Chris Blume <cblume@chromium.org>
* 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>
* Mark SkGifImageReader destructors with overrideGravatar Kevin Lubick2017-05-24
| | | | | | | | | | | Very new versions of Clang (5.0 TOT) call out this problem. Bug: skia: Change-Id: If90f658f258ca52ebb21e2352fbf3180c41008bd Reviewed-on: https://skia-review.googlesource.com/17795 Commit-Queue: Kevin Lubick <kjlubick@google.com> Commit-Queue: Mike Klein <mtklein@chromium.org> Reviewed-by: Mike Klein <mtklein@chromium.org>
* 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>
* 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>
* 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>
* Simplify subset computation in SkGifImageReaderGravatar Leon Scroggins III2017-04-07
| | | | | | | | | | | | If xOffset or yOffset was not zero, these computations would not be evaluated anyway, so no need to add zero for these comparisons. (Split off from CL 9810) Change-Id: I8481eb257d96766696945cef4eafaff4281fb7d9 Reviewed-on: https://skia-review.googlesource.com/11782 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* GIF: Only report a frame after knowing dependencyGravatar Leon Scroggins III2017-01-30
| | | | | | | | | | | | | | | | | | | | Previously, getFrameInfo might report a frame that was truncated prior to setting its requiredFrame. As a result, fRequiredFrame may be different depending on how much data has already been received. If there is a local color table, do not report the frame until the color table has been received, since that is used to determine fRequiredFrame. If there is no local color table, set fRequiredFrame and report the frame after reading the header. Add a test. Replace make_from_resource with GetResourceAsData Change-Id: I1b697f766c1d0e1e12ab2ae1d27167af5193395d Reviewed-on: https://skia-review.googlesource.com/7756 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@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>
* Make SkGIFLZWBlock modifiable so it is assignableGravatar Chris Blume2016-12-21
| | | | | | | | | | | | | std::vector needs to be able to assign objects contained inside it. With const member variables, this isn't possible. Remove the consts so SkGIFLZWBlock can be assigned. BUG=skia:6072 Change-Id: I990dc80fb1c49fbd584712c6d0c1154c2da36e85 Reviewed-on: https://skia-review.googlesource.com/6362 Reviewed-by: Leon Scroggins <scroggo@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* GIF: Avoid copying/storing data when possibleGravatar Leon Scroggins III2016-12-19
| | | | | | | | | | | | | | | | | | | If the input SkStream has a length and position, do not copy and store LZW blocks or ColorMaps. Instead, mark the position and size, and read from the stream when necessary. This will save memory in Chromium's use case, which has already buffered all of its data. In the case where we *do* need to copy, store it on the SkStreamBuffer. This allows SkGifImageReader to have simpler code. Add tests. Change-Id: Ic65fa766328ae2e5974b2084bc2099e19aced731 Reviewed-on: https://skia-review.googlesource.com/6157 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* Do not create SkGifCodec if true size is not knownGravatar Leon Scroggins III2016-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If there is enough data in the stream to read the reported canvas size, but not enough to read the first image's header, we do not know the true canvas size, since we may expand it to fit the first frame. In that case, return nullptr from NewFromStream. Add a test. SkGifCodec.cpp: Correct a comment - parse returns false if there is a fatal error. parse() returning true does not guarantee that the size was found. Instead of checking the width and height, check to see whether the first frame exists and has its header defined. If not, we do not yet know the true canvas size. Assert that the canvas size is non-zero, which is a fatal error from parse. SkGifImageReader.cpp: Move the code to set the header defined before the SkGIFSizeQuery exit condition. This allows SkGifCodec to check the first frame's header to determine whether the size is known. GifTest.cpp: Add a test which truncates the file just before the image header (and after the global header). Prior to the other changes, this would create an SkCodec. For an image that needs its canvas size expanded, the SkCodec would have an incorrect size. CodecPartialTest.cpp: randPixels.gif now needs more than half of its data to create an SkCodec, so set a minimum for test_partial. Change-Id: I40482f524128b2f1fe59b8f27dd64c7cbe793079 Reviewed-on: https://skia-review.googlesource.com/5701 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com>
* GIF: Internal cleanup - remove color map parameterGravatar Leon Scroggins III2016-12-05
| | | | | | | | | | | | | SkGIFFrameContext::decode() and SkGIFLZWContext::prepareToDecode() do not need (or use) the global color map, so stop passing it as a parameter. The parameter was used prior to https://skia-review.googlesource.com/c/4379/ (different issue!), but we overlooked removing it then. Change-Id: I0f477e9db11f7650938d6b868baef69e3b37d86b Reviewed-on: https://skia-review.googlesource.com/5609 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: Matt Sarett <msarett@google.com>
* Write transparent pixels more often in SkGifImageReaderGravatar Matt Sarett2016-11-03
| | | | | | | | | | | | | | | | | | | | | | | | | This stems from a behavior difference between Skia and Chrome. In Skia, we want to write transparent pixels as often as possible. (It's faster than checking if we should skip each pixel.) In Chrome, they avoid writing transparent pixels unless absolutely necessary. We were cautious about changing behavior when this first landed, but this is easier to think about in a smaller change (right now). (1) We can always write transparent pixels when we are writing an independent frame. (2) There is no need for the progressiveDisplay() check. We only ever use progressive display methods on the first frame - and the first frame is always independent. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4379 Change-Id: I82048a08e2003aac216f483c7db8df997b687149 Reviewed-on: https://skia-review.googlesource.com/4379 Commit-Queue: Matt Sarett <msarett@google.com> Reviewed-by: 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
* Treat a GIF with no color table as transparentGravatar scroggo2016-10-31
| | | | | | | | | | | | | | | When checking to see whether a GIF has transparency to determine its alpha type, treat an empty color table as having alpha, since we will draw it as a transparent image. (This is a separate bug from skbug.com/5883, but the image I used to verify that bug was drawn to 565 as black. The fix is to not support 565 in that case, by changing its recommended alpha type.) BUG=skia:5883 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2461813002 Review-Url: https://codereview.chromium.org/2461813002
* Fix decoding GIF to 565Gravatar scroggo2016-10-27
| | | | | | | | | | | | 565 cannot take the !writeTransparentPixels path, so disable it for cases where we might have to take that path. This only affects frames beyond the first. If the first frame has a transparent pixel, it will be marked as non-opaque, so we cannot decode to 565 anyway. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2441833002 Review-Url: https://codereview.chromium.org/2441833002
* Write transparent pixels more often (SkGifCodec)Gravatar scroggo2016-10-26
| | | | | | | | | | | | | | | | | | Writing transparent pixels is faster than the alternative, and we can skip clearing the frame to transparent. We'll still clear if the image is incomplete. I ran ./out/Release/nanobench --images <images> --samples 100 --sourceType image --simpleCodec -v over the GIFs we have on our bots, and found an average ~13% speedup. Raw data is on sheet 2 of https://docs.google.com/spreadsheets/d/19V-t9BfbFw5eiwBTKA1qOBkZbchjlTC5EIz6HFy-6RI/ (the sheet is named WriteTransparentPixels). GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2436183002 Review-Url: https://codereview.chromium.org/2436183002
* Fix some Windows warningsGravatar Jim Van Verth2016-10-26
| | | | | | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3980 Change-Id: Icfc5dfb985b966c625d9bc81f61719ac5549085e Reviewed-on: https://skia-review.googlesource.com/3980 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Jim Van Verth <jvanverth@google.com>
* Fix more namespace conflicts in SkGifImageReaderGravatar scroggo2016-10-25
| | | | | | | | | | To fix Google3 TBR=benjaminwagner@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2450753003 NOTREECHECKS=true Review-Url: https://codereview.chromium.org/2450753003
* Move third_party/gif's license into its own fileGravatar scroggo2016-10-24
| | | | | | GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2448573002 Review-Url: https://codereview.chromium.org/2448573002
* Rename GIFImageReader to SkGifImageReaderGravatar scroggo2016-10-24
| | | | | | | | The former could violate One Definition Rule in Google3, since other projects that are based on Chrome/webkit also have GIFImageReader. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2445653004 Review-Url: https://codereview.chromium.org/2445653004
* 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-05-19 18:01:54 +0000