Skip to content

M116 public#196

Merged
kyamagu merged 185 commits intoskia-python:mainfrom
HinTak:m116-public
Aug 8, 2023
Merged

M116 public#196
kyamagu merged 185 commits intoskia-python:mainfrom
HinTak:m116-public

Conversation

@HinTak
Copy link
Collaborator

@HinTak HinTak commented Aug 2, 2023

As the title says. I disabled about two tests which are too broken (segfaults) but the rest is 113 failed, 1974 passed, 34 skipped and 8 xfailed, 66 errors.

You may need to adjust the non-linux build instructions.

I only have one package which depends on skip-python and this works okay as a drop-in for m87.

I don't know how far/fast I can do the rest, but this is useful as is.

I have a detached mirror called skia-m1xx-python to add a few more interesting stuff for my own use; file issues and track progress, etc.

HinTak added 30 commits July 30, 2023 01:06
Blindly substitute "Bitmap.cpp" with "#include <include/core/SkBitmap.h>" etc
for all current files.

Adjustments to actually start to build
‘SkSVGDOM::SkSVGDOM(sk_sp<SkSVGSVG>, sk_sp<SkFontMgr>, SkSVGIDMapper&&)’ is private in m88.

Like-wise, in m116 header:
    SkSVGDOM(sk_sp<SkSVGSVG>, sk_sp<SkFontMgr>, sk_sp<skresources::ResourceProvider>,
        SkSVGIDMapper&&);
is private.

There is a symbol in the shared library, which presumably should not be used:
    SkSVGDOM::SkSVGDOM(sk_sp<SkSVGSVG>, sk_sp<SkFontMgr>, sk_sp<skresources::ResourceProvider>,
        skia_private::THashMap<SkString, sk_sp<SkSVGNode>, SkGoodHash>&&)
Adjusting paths
…dingContext; programmatic changes.

perl -pi -e 's(GrContext\*)(GrDirectContext*)g' src/skia/*
perl -pi -e 's(GrContext::)(GrDirectContext::)' src/skia/GrContex*
perl -pi -e 's(GrContext\&)(GrDirectContext\&)g' src/skia/GrContex*
chrome/m116:RELEASE_NOTES.md:

Milestone 113
-------------
  * The define SK_SUPPORT_GPU is now SK_GANESH. It is no longer detected as a 0 or 1, but
    as the absence or presence of that define. As a result, it defaults to off (not defined) if
    not defined (SK_SUPPORT_GPU would default to SK_SUPPORT_GPU=1 if not defined).
m104 has overloaded SkBitmap::erase():
    void erase(SkColor4f c, const SkIRect& area) const;
    void erase(SkColor c, const SkIRect& area) const;
Milestone 113:

  * SkImage factory methods have been moved to the SkImages namespace. Many have been renamed to
    be more succinct or self-consistent. Factory methods specific to the Ganesh GPU backend are
    defined publicly in include/gpu/ganesh/SkImageGanesh.h.
      * SkImage::MakeBackendTextureFromSkImage -> SkImages::GetBackendTextureFromImage
      * SkImage::MakeCrossContextFromPixmap -> SkImages::CrossContextTextureFromPixmap
      * SkImage::MakeFromAdoptedTexture -> SkImages::AdoptTextureFrom
      * SkImage::MakeFromBitmap -> SkImages::RasterFromBitmap
      * SkImage::MakeFromCompressedTexture -> SkImages::TextureFromCompressedTexture
      * SkImage::MakeFromEncoded -> SkImages::DeferredFromEncodedData
      * SkImage::MakeFromGenerator -> SkImages::DeferredFromGenerator
      * SkImage::MakeFromPicture -> SkImages::DeferredFromPicture
      * SkImage::MakeFromRaster -> SkImages::RasterFromPixmap
      * SkImage::MakeFromTexture -> SkImages::BorrowTextureFrom
      * SkImage::MakeFromYUVAPixmaps -> SkImages::TextureFromYUVAPixmaps
      * SkImage::MakeFromYUVATextures -> SkImages::TextureFromYUVATextures
      * SkImage::MakePromiseTexture -> SkImages::PromiseTextureFrom
      * SkImage::MakePromiseYUVATexture -> SkImages::PromiseTextureFromYUVA
      * SkImage::MakeRasterCopy -> SkImages::RasterFromPixmapCopy
      * SkImage::MakeRasterData -> SkImages::RasterFromData
      * SkImage::MakeRasterFromCompressed -> SkImages::RasterFromCompressedTextureData
      * SkImage::MakeTextureFromCompressed -> SkImages::TextureFromCompressedTextureData
    To help in the transition, there is some temporary bridge code (e.g. aliases) which will
    eventually be removed.
commit bab392fd3d9a48e0b696d1799d5271444c199643
Author: Kevin Lubick <[email protected]>
Date:   Tue Apr 11 13:51:02 2023 -0400

    Deprecate SkImage::encodeToData and migrate all internal uses

    By removing this method, we will further (completely?) decouple
    SkImage from our encoders, allowing for them to not contribute
    to code size when not needed.

    This adds replacement functions to include/encode/Sk*Encoder.h.

    This CL must land with the follow-on CL to preserve the ability
    to use NDK encoders instead of bundling in ones based on libpng
    et al.

    Change-Id: Ie20a41cfdf65c975f640fb9368f363a588bd9394
    Bug: skia:13983
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/667296
    Reviewed-by: Herb Derby <[email protected]>
    Reviewed-by: Leon Scroggins <[email protected]>
commit 8a0152a423497497cc7425541947cf1bb3745a2e
Author: Kevin Lubick <[email protected]>
Date:   Mon Jun 5 11:41:39 2023 -0400

    Make SkShaders a namespace and move SkPerlinNoiseShader functions into it

    I want to reclaim SkPerlinNoiseShader in src/shaders when refactoring
    it to remove asFragmentProcessor.

    This also enforces IWYU on three public shader headers.

    Client CLs:
     - http://ag/23535765
     - https://crrev.com/c/4584205
     - http://cl/537870282

    Canary-Chromium-CL:4584205
    Change-Id: I9a09f1aa6b2b58fd40e41e74b4a290e75699c220
    Bug: skia:13052, skia:14317
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/706539
    Reviewed-by: Brian Osman <[email protected]>
    Commit-Queue: Kevin Lubick <[email protected]>
commit ec87dc1b767efabd1e204eef66eaa6df102eb2e8
Author: Mike Reed <[email protected]>
Date:   Thu May 20 15:16:34 2021 -0400

    Just expose factories for patheffects

    bug: skia:11957
    Change-Id: If2983fcd1b520a7ae77650d7e5ab226af9db52e0
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410782
    Commit-Queue: Mike Reed <[email protected]>
    Reviewed-by: Tyler Denniston <[email protected]>
Milestone 88
------------

  * Removed SkSurfaceProps::kLegacyFontHost_InitType, SkFontLCDConfig, and related code.
    The default pixel geometry for SkSurfaceProps is now kUnknown instead of kRGB_H.
    The removal was guarded by the SK_LEGACY_SURFACE_PROPS build flag which was later removed.
    https://review.skia.org/322490
    https://review.skia.org/329364
@HinTak HinTak requested a review from 0lru August 5, 2023 19:43
The build script expects to be able to patch it, so we need to unpatch
at end of pre-fetch. Teach me not to put "obviously" stuff in last
minute before release again!
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 6, 2023

@kyamagu @0lru all checks passed. The final count is that I have disabled about 80 tests with "m116:REVISIT", another 30 with "may not need REVISIT" and two "segfault:REVISIT". Out of about 2200 tests. Considering about 20% of API changed between m87 and m116, I may have fixed up about 15% of those and brought it up to about 5% broken/removed/disabled. That sounds about right. Just need an approving review to release.

My detached mirror's CI is functional - it only kicked in finally when I made the 116.0b1 release (and failing at the publishing stage, quite understandably). That means I can try things out with a pull there on platform I don't have, before putting it here, for future updates...if there is any :-(.

Copy link
Collaborator

@kyamagu kyamagu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I've left just a few comments.

@kyamagu
Copy link
Collaborator

kyamagu commented Aug 7, 2023

@HinTak Just to confirm one more, have you checked the documentation build? Might be good to 1) minimally update the README.md, and 2) include README.m116.md in a new section of the docs/, like release notes, changes, or migration guide.

@HinTak
Copy link
Collaborator Author

HinTak commented Aug 7, 2023

@HinTak Just to confirm one more, have you checked the documentation build? Might be good to 1) minimally update the README.md, and 2) include README.m116.md in a new section of the docs/, like release notes, changes, or migration guide.

The build docs part of CI passed. I actually haven't touched the docs directory at all. TL;DR, I think as far as migration goes, some needs a "skis.SamplingOption" inserted, a even smaller set of people will notice planarConfig becomes two. Don't know how many will notice things gone and missed them - hence the "attributeerror" mention.

@HinTak
Copy link
Collaborator Author

HinTak commented Aug 7, 2023

I am staging some updates at HinTak/skia-m1xx-python#1 . This allows me to cancel etc, without polluting your workflow too much. HEAD over there is identical to this pull, so pull-1 is update to this (via cherry-pick eventually, collapsing history and removing wrong turns, etc), except the 2nd commit, which enables CI over there, which will never be cherry-pick'ed into this.

HinTak added 13 commits August 8, 2023 00:46
Milestone 116
-------------
  * `SkImageFilters::AlphaThreshold` has been removed. Its only use was in ChromeOS and that usage has been replaced with a `Blend(kSrcIn, input, Picture(region))` filter graph to achieve the same effect.
…eam release note.

Milestone 116
-------------
  * `SkShaders` is now a namespace (was previously a non-constructable class with only static
    functions). `SkPerlinNoiseShader::MakeFractalNoise` and `SkPerlinNoiseShader::MakeTurbulence` have
    been moved to the `SkShaders` namespace and `SkPerlinNoiseShader` (the public non-constructable
    class) has been slated for moving into private internals of Skia.
    There are no functional differences in the moved functions, however the change of some #includes
    in `include/core/SkShader.h`, `include/effects/SkGradientShader.h`, and
    `include/effects/SkPerlinNoiseShader.h` may cause clients who were depending on the transitive
    dependencies to now fail to compile.
Milestone 116
-------------
  * The deprecated `SkTableColorFilter` class and its methods have been removed. Clients should use
    `SkColorFilters::Table` and `SkColorFilters::TableARGB` (defined in include/core/SkColorFilter.h).
…)` constructor has been removed from the public API.

Milestone 116
-------------
  * The `SkYUVAPixmapInfo::SupportedDataTypes(const GrImageContext&)` constructor has been removed from
    the public API.
…ePictureSnapshot

Milestone 115
-------------
  * `SkDrawable::newPictureSnapshot` is removed. Instead, call `SkDrawable::makePictureSnapshot`.
    The old method returned a bare (but ref-counted) pointer, which was easy for clients to get wrong.
    The new method returns an `sk_sp<SkPicture>`, which is easier to handle, and consistent with the
    rest of skia.
Milestone 111
-------------
  * SkBackingFit is no longer part of the public API.
…thWithPaint

Milestone 110
-------------
  * SkPaint::getFillPath has been replaced with skpathutils::FillPathWithPaint from
    include/core/SkPathUtils.h. The functionality should be the same.

So the m110 release note was incorrect/incomplete: there is a new "const SkPaint&" argument

Mis-read the prototype - the arguments are re-arranged

Typo

Re-enable test on SkPaint::getFillPath emulation
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 8, 2023

@kyamagu bunch of small changes, typos, a few "revisit" switched to xfail when there is a upstream release note item, and a few "revisit" removed with the corresponding code re-enabled/changed to work. Bumped to 116.0b2 . I think this is it. I think the next one is probably 117.0b2 as m117 should be out any time now :-(.

@kyamagu kyamagu requested review from kyamagu and removed request for 0lru August 8, 2023 05:04
Copy link
Collaborator

@kyamagu kyamagu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Perhaps we should sort out the documentation in the next release

@kyamagu kyamagu merged commit f71fdee into skia-python:main Aug 8, 2023
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 9, 2023

Thanks. Btw, the aarch64 build is taking close to 5.5 hours in some runs, so it might need to be split into two at some point. (I read it being special was because of a 6 hour limit). It might go over 6 at some point. One idea is to split the gn build which takes 1/2 hour into its own task. There is also the fact that it is the first and take 5 hours - would be nice to know if something is trivially broken (typo/missing braces) earlier, so it would be nice to move it later, or even it being triggered conditionally only occasionally. Anyway, I'll think about that and may try some ideas in my detach mirror.

@kyamagu
Copy link
Collaborator

kyamagu commented Aug 9, 2023

@HinTak Agreed. Perhaps it is possible to split the CI jobs into the following steps (within cibuildwheel containers), or even into a different repository dedicated to Skia build.

  1. Build Skia dependencies
  2. Build Skia
  3. Build wheels

@HinTak
Copy link
Collaborator Author

HinTak commented Aug 9, 2023

I checked my CI logs, the worst was 5 hours 52 minutes. :-(. Splitting gn out will split out only about half an hour. But ninja is clever enough to be interrupted / by stages - so it is possible to do ninja -C out/Release third_party/dng_sdk etc before the default target. That makes build_Linux.sh quite complicated - build gn, build 3rd-party, build skia.

Having a build 3rd-party would really help: icu and harfbuzz are both quite large, and 3rd party adds about 700 compilations to a total of 1900 I think. So having a ninja -C out/Release third_party/* in the middle should really help to cut the compilation time of skia by 1/3.

@HinTak HinTak deleted the m116-public branch May 22, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants