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

M117 public #197

Merged
merged 46 commits into from
Sep 22, 2023
Merged

M117 public #197

merged 46 commits into from
Sep 22, 2023

Conversation

HinTak
Copy link
Collaborator

@HinTak HinTak commented Aug 11, 2023

As the name says. M117 is out. The changes are relatively small and "obvious" (compared to m87 to m116...). The most worrying aspect is that surface.flushandsubmit and surface.flush are gone. I don't know if something else needs to be done. But I would recommend NOT merging this until that's investigated.

Canvas.flush is now emulated - but they actually tell you want to do in the m117 release note.

Also the aarch64 pre job is now split into two - one compiles 800 files for bundled 3rd party and another for skia proper, about 1200 files. Since worst case is about 5 hours 40 minutes, this split the worst case down to about 2 and 4. (Usually it is more like 1.5 hours and 2.5 hours, to make a total of 4).

Anyway, I am thinking of shifting that either sideway or behind the wheel matrix ie split the wheel matrix into native builds vs qemu builds, and re-order them into native builds, skia 3rd party, skia proper, and qemu builds, docs etc. It is more useful to see the result of native build earlier, for failures.

This should pass CI.

$ git describe
canvaskit/0.38.2-1014-g26ce945468
m117: <include/core/SkSurfaceCharacterization.h> no longer available

commit 0bff57e0fe948bd9d96e92960e0ca993629c9a28
Author: Kevin Lubick <kjlubick@google.com>
Date:   Fri Jun 9 14:29:17 2023 -0400

    Move DDL and SkSurfaceCharacterization to include/private/chromium

    This also renames them to be more clearly Ganesh-specific:
     - SkDeferredDisplayList -> GrDeferredDisplayList
     - SkDeferredDisplayListRecorder -> GrDeferredDisplayListRecorder
     - SkSurfaceCharacterization -> GrSurfaceCharacterization

    SkSurface::draw(sk_sp<GrDeferredDisplayList>...) has been moved
    to a static function - skgpu::ganesh::DrawDDL. This was better than
    re-writing the API to take a GrDeferredDisplayList* which would
    require re-wrapping in a sk_sp to be passed into the deeper layers -
    an error-prone and efficient strategy.

    Shim headers are still around, as well as a shim over the old
    SkSurface method until Chromium can be migrated.

    This CL has no functional changes in how DDLs work, besides
    removing the ability to specify an offset, which was not
    implemented and there are no plans to do so.

    Change-Id: Ibb263d6b90e3b3616ffd11c7a03b69e78dd02fe4
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/706017
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
    Owners-Override: Kevin Lubick <kjlubick@google.com>
    Reviewed-by: Robert Phillips <robertphillips@google.com>
Milestone 117
-------------
  * `SkCanvas::flush()` has been removed. It can be replaced with:
    ```
        if (auto dContext = GrAsDirectContext(canvas->recordingContext())) {
            dContext->flushAndSubmit();
        }
    ```
commit e3ca856e7ed6c285734dacf87faccdbf9a321f05
Author: Kevin Lubick <kjlubick@google.com>
Date:   Wed Jul 26 08:58:05 2023 -0400

    Remove legacy SkImage and SkSurface methods

    Client CLs:
     - http://ag/23168890
     - http://ag/23171851
     - http://ag/23473641
     - http://ag/23494824
     - http://cl/551209733
     - https://crrev.com/c/4566404
     - https://crrev.com/c/4705004
     - flutter/engine#42425
     - flutter/engine#43786
     - flutter/engine#43965

    Change-Id: I06c71cf6dc77aeaa3f78dec61c7b7c6eca688884
    Bug: b/40045064
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/729776
    Reviewed-by: Michael Ludwig <michaelludwig@google.com>
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
    Reviewed-by: Brian Osman <brianosman@google.com>
commit f437d01005173a284d1e9c592bd0c61360a6cad1
Author: Kevin Lubick <kjlubick@google.com>
Date:   Mon Jun 26 14:35:06 2023 -0400

    Remove slug-related #ifdefs from src/core

    This also enforces IWYU on some of the affected types
    SkPicture etc.

    I made SkPicturePlayback a friend of SkCanvas so the former
    could call SkCanvas::draw(Slug) directly without needing to
    have the implementation be linked in in a CPU build.

    I added a null-returning implementation for deserialization
    because there is still a connection between SlugImpl -> SubRunContainer
    -> Ganesh code that means we cannot quite move slugs into
    a CPU only build

    Canary-Chromium-CL: 4649851
    Bug: skia:14317
    Change-Id: I98f776596288e2f6f3266549f5c9c8d12e9ba551
    Reviewed-on: https://skia-review.googlesource.com/c/skia/+/715416
    Commit-Queue: Kevin Lubick <kjlubick@google.com>
    Reviewed-by: Herb Derby <herb@google.com>
Mention SkCanvas::flush() is emulated
…er" on aarch64

build_Linux.sh has also been adjusted/re-arranged to insist on being
re-run on aarch64.
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 11, 2023

Windows build is running out of space. I wonder what it requires...

…ure on windows

Trying to avoid windows running out of disk space.
HinTak and others added 16 commits August 14, 2023 18:27
…ing code just before removal

Can't call other surface.flush() method either; use direct

A lot of typos and missing namespaces

typo
Milestone 114
-------------
  * `SkImages::GetBackendTextureFromImage` has been renamed `SkImages::MakeBackendTextureFromImage`.
  * `SkImage::getBackendTexture()` has been moved to `SkImages::GetBackendTextureFromImage()` in
    `SkImageGanesh.h`.
  * `SkImage::makeTextureImage()` has been moved to `SkImages::TextureFromImage()` in
    `SkImageGanesh.h`.
  * `SkImage::flush()` and `SkImage::flushAndSubmit()` has been moved to
    `GrDirectContext::flush()` and `GrDirectContext::flushAndSubmit()` in `SkImageGanesh.h`.
…bmit()`

Milestone 114
-------------
  * `SkImage::flush()` and `SkImage::flushAndSubmit()` has been moved to
    `GrDirectContext::flush()` and `GrDirectContext::flushAndSubmit()` in `SkImageGanesh.h`.
…Image()`

    Milestone 114
    -------------
      * `SkImage::getBackendTexture()` has been moved to `SkImages::GetBackendTextureFromImage()` in
        `SkImageGanesh.h`.
      * `SkImage::makeTextureImage()` has been moved to `SkImages::TextureFromImage()` in
        `SkImageGanesh.h`.
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 21, 2023

This skips about 10 fewer tests than 116b2 at this point , so is a marginal improvement.

It appears that there are some subtle changes separating GPU-backed images vs
CPU-backed images, and GPU-backed images can't be compressed/saved directly.

Probably related to this:

m113 Bulk changes for SkImage->SkImages

commit 77472bf8434041b6e8a239f93816088e680f479c
Author: Kevin Lubick <kjlubick@google.com>
Date:   Fri Mar 24 07:11:17 2023 -0400

    Reland "Reland "Introduce SkImages namespace; remove Ganesh GPU code from SkImage_Raster""

    This reverts commit 3e6bfdfea566b8368b10cd911499ae1bb09d3004.

    Reason for revert: Fixing Android
…tureSize() instead.

Milestone 90
------------
  * GrDirectContext::ComputeImageSize() is removed. Use SkImage::textureSize() instead.
    https://review.skia.org/368621
    https://review.skia.org/369317
    https://review.skia.org/371958

static method and shared-pointers
@HinTak
Copy link
Collaborator Author

HinTak commented Aug 27, 2023

Failing with "error: invalid command 'build_sphinx'". That seems to be a problem elsewhere?

@HinTak
Copy link
Collaborator Author

HinTak commented Sep 17, 2023

M118 is out. I 'll write a bit more in readme.m117 and bump it to 117b3 soon. Slightly undecisive about whether to merge readme.m117 or rename / prepend readme.m116, but probably less important as long as something is written down... anyway.

@HinTak
Copy link
Collaborator Author

HinTak commented Sep 22, 2023

Bumped up to v117b3, and updated Readme
M117. Should be ready to go.

(More breakage on m118. Sigh...)

@HinTak
Copy link
Collaborator Author

HinTak commented Sep 22, 2023

@kyamagu ready to go.

@HinTak
Copy link
Collaborator Author

HinTak commented Sep 22, 2023

New contributor @jonathanhogg , thanks.

Copy link
Owner

@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.

Would request a small change in the env variable name

.github/workflows/ci.yml Show resolved Hide resolved
@HinTak HinTak merged commit d13a801 into kyamagu:main Sep 22, 2023
13 checks passed
@kyamagu
Copy link
Owner

kyamagu commented Sep 22, 2023

@HinTak You can create a release on Github

@HinTak
Copy link
Collaborator Author

HinTak commented Sep 23, 2023

@kyamagu thanks - think it is done, going through CI.

@HinTak HinTak deleted the m117-public branch May 22, 2024 18:42
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.

None yet

3 participants