Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skia m122 core support #199

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

HinTak
Copy link

@HinTak HinTak commented Feb 2, 2024

Description

Fixes compatibility with newer skia (tested with m122).

Related Issue

Addresses #187 , supercedes #189 . This is basically an update of #189 by splitting it more logically into two parts, this and #198 .

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

commit 5c93acf313d1cf72f8b73886168bd925b5011b8c
Author: Kevin Lubick <kjlubick@google.com>
Date:   Tue May 9 12:11:43 2023 -0400

    Move SkSurface factories to SkSurfaces namespace

      * SkSurface::MakeFromAHardwareBuffer -> SkSurfaces::WrapAndroidHardwareBuffer
      * SkSurface::MakeFromBackendRenderTarget -> SkSurfaces::WrapBackendRenderTarget
      * SkSurface::MakeFromBackendTexture -> SkSurfaces::WrapBackendTexture
      * SkSurface::MakeFromCAMetalLayer -> SkSurfaces::WrapCAMetalLayer
      * SkSurface::MakeFromMTKView -> SkSurfaces::WrapMTKView
      * SkSurface::MakeGraphite -> SkSurfaces::RenderTarget
      * SkSurface::MakeGraphiteFromBackendTexture -> SkSurfaces::WrapBackendTexture
      * SkSurface::MakeRaster -> SkSurfaces::Raster
      * SkSurface::MakeRasterDirect -> SkSurfaces::WrapPixels
      * SkSurface::MakeRasterDirectReleaseProc -> SkSurfaces::WrapPixels
      * SkSurface::MakeRasterN32Premul -> SkSurfaces::Raster
      * SkSurface::MakeRenderTarget -> SkSurfaces::RenderTarget

    Suggested review order:
     - include/*
     - src/gpu/ganesh/surface/SkSurface_Ganesh.cpp
     - src/image/SkSurface_Raster.cpp
     - src/image/SkSurface.cpp
     - All other changes which were mostly find-replace followed
       by `git clang-format origin/main`

    Change-Id: Idb18ab5c2beb12d8b4ec6712e9abee286646424f
    Bug: skia:13983
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/687639
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
    Reviewed-by: Brian Osman <brianosman@google.com>
commit d21c3f85a2425d7d31ae1a258b920fccd53aed1c
Author: Kevin Lubick <kjlubick@google.com>
Date:   Fri Mar 17 18:04:20 2023 -0400

    Introduce SkImages namespace; remove Ganesh GPU code from SkImage_Raster

    This is the first of many CLs that aims to split the GPU and CPU
    coupling to SkImage.

    Step 1 was to move all SkImage::Make static functions (factories)
    to their own namespace (SkImages), which allows us to define these
    in different header files, which can be optionally exposed via the public API.

      * 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

    We chose SkImages as the name because it aligns with existing
    "effective namespaces" like `SkShaders` and `SkColorFilters`.

    Step 2 was to move the logic requiring the Ganesh GPU backend out of
    SkImage classes (starting with SkImage_Raster as an easy starting point)
    and into files that live under src/gpu/ganesh. The graphite code
    will come later. This involved making SkImage_Raster.h so ganesh
    can see that subclass and exposing a few fields via getters/setters.

    I don't think these steps can be easily split into two CLs (sorry).
    I've left backwards compatible code so we can fix up clients
    incrementally (and lock it in via the
    SK_DISABLE_LEGACY_IMAGE_FACTORIES define).

    I ran `git clang-format origin/main` after the renaming, and spot
    checked the formatting; seemed fine to me.

    Suggested review order:
     - SkImage.h, SkImageGanesh.h, SkImageAndroid.h. Note that SkImage.h
       has all the legacy factories guarded by an #ifdef at the bottom
       for easier deletion in the future.
     - src/image/SkImage_*Factories.cpp to see where the implementation
       of these factories have been moved. We want them in separate files
       to make the modular builds easier in the future.
     - SkImage_Base.cpp, SkImage_Raster.h where things were moved out
       of SkImage.cpp and SkImage_Raster.cpp for better organization.
     - GrImageUtils.cpp/.h to see where the other methods of SkRaster
       that used the GPU code were moved. I hope to follow suit with
       other Images but for now just keep their onAsView etc methods
       in place.
     - Many other changes were mechanical to use the new namespace
       and methods.

    Change-Id: Ia345c2f0f1b1a2f65178a857bdd0d6118fee7e58
    Bug: skia:13983
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/648297
    Reviewed-by: Brian Osman <brianosman@google.com>
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
commit 539fb10d7cfb3e73ae43bdedba7d11b2012f5446
Author: Kevin Lubick <kjlubick@google.com>
Date:   Thu May 11 13:54:20 2023 -0400

    Move SkSurface::MakeNull to SkSurfaces::Null

    Follow-up to https://skia-review.googlesource.com/c/skia/+/687639

    Change-Id: I19c01a0c3957460d6c79c9edb2ddabd4c8c2d2cf
    Bug: skia:13983
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/696537
    Reviewed-by: Brian Osman <brianosman@google.com>
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
Shared library build of Skia has "SkDebugf(char const*, ...)" in a separate static library.

$ nm -C ../../../../skia/out/Shared/libpathkit.a | grep SkDebugf
...
0000000000000000 T SkDebugf(char const*, ...)
@HinTak HinTak mentioned this pull request Feb 2, 2024
10 tasks
@HinTak HinTak closed this Feb 2, 2024
@HinTak HinTak reopened this Feb 2, 2024
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.

1 participant