Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

Shader warmup #37662

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Shader warmup #37662

merged 1 commit into from
Nov 16, 2022

Conversation

schwa423
Copy link
Contributor

@schwa423 schwa423 commented Nov 16, 2022

This PR fixes illegal behavior in the ~VulkanSurface() destructor, which invokes a std::function without first checking if it is empty; it will be empty if nobody calls VulkanSurface::SetReleaseImageCallback(). This is true in the case of Skia shader warmup on Fuchsia.

This CL fixes the following issue: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=115400

Note: the root cause is not directly related to shader warmup, and could potentially manifest in other situations (not after this PR is merged though).

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@@ -184,7 +184,7 @@ VulkanSurface::~VulkanSurface() {
if (buffer_id_) {
session_->DeregisterBufferCollection(buffer_id_);
}
} else {
} else if (release_image_callback_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change lgtm. But we should clean up the other files from the pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased change onto main, now there's only 1 line changed in 1 file.

Before, ~VulkanSurface() invokes a potentially empty std::function.
Now, checks if it is empty before invoking it.
@uysalere uysalere merged commit f4ca3e7 into flutter:main Nov 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 16, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 16, 2022
…115478)

* 7a5bc9124 Roll Skia from 4b3d36f36bf3 to 74a57221dfb3 (7 revisions) (flutter/engine#37675)

* f4ca3e7a0 [fuchsia] Fix shader warmup. (flutter/engine#37662)

* 97974ebb5 Fix resize crash in Android virtual display (flutter/engine#37329)
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
…lutter#115478)

* 7a5bc9124 Roll Skia from 4b3d36f36bf3 to 74a57221dfb3 (7 revisions) (flutter/engine#37675)

* f4ca3e7a0 [fuchsia] Fix shader warmup. (flutter/engine#37662)

* 97974ebb5 Fix resize crash in Android virtual display (flutter/engine#37329)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…lutter#115478)

* 7a5bc9124 Roll Skia from 4b3d36f36bf3 to 74a57221dfb3 (7 revisions) (flutter/engine#37675)

* f4ca3e7a0 [fuchsia] Fix shader warmup. (flutter/engine#37662)

* 97974ebb5 Fix resize crash in Android virtual display (flutter/engine#37329)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants