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

Reland fix wrong VSYNC event #37865

Merged
merged 12 commits into from
Dec 7, 2022
Merged

Reland fix wrong VSYNC event #37865

merged 12 commits into from
Dec 7, 2022

Conversation

fzyzcjy
Copy link
Contributor

@fzyzcjy fzyzcjy commented Nov 23, 2022

Reland #36775
Previously reverted by #37589
Close flutter/flutter#115161

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.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@chinmaygarde
Copy link
Member

@iskakaushik For a review as someone who approved the previous patch. Dan is OOO.

@iskakaushik
Copy link
Contributor

@fzyzcjy can you please describe why it broke before and how this patch fixes it? Sorry for the slow response on this.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Dec 5, 2022 •
edited
Loading

@iskakaushik Hi, it is described in: #36775 and flutter/flutter#115161

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Dec 5, 2022

Shortly speaking, the old way may generate events that violate tracing spec because the time is synthesized. The new way does not have this problem, because it does not generate any fake time for events, but only add arguments to events.

@iskakaushik
Copy link
Contributor

@kenzieschmoll does devtools rely on the presence of a standard VSYNC event?

@kenzieschmoll
Copy link
Member

We don't do any matching on an event named 'VSYNC' in DevTools, no. Thank you for checking.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Dec 7, 2022

CI passes

iskakaushik
iskakaushik approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

LGTM

@iskakaushik iskakaushik added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 7, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 7, 2022

auto label is removed for flutter/engine, pr: 37865, due to - Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 7, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 7, 2022

auto label is removed for flutter/engine, pr: 37865, due to Validations Fail.

@iskakaushik iskakaushik merged commit 8f6036e into flutter:main Dec 7, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 9, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 9, 2022
@engine-flutter-autoroll
8f6036e58 Reland fix wrong VSYNC event (flutter/engine#37865)
9b935f4
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 9, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 8d83b98c55b3 to 030950f3070c (29 revisions) (#…
86fa9e5 
…116802)

* bd8bcf956 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120)

* dec8b5221 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754)

* 5545ccf87 Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122)

* 80a15a419 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963)

* 5caef8585 Full implementation of text-input-test (flutter/engine#37986)

* 8f6036e58 Reland fix wrong VSYNC event (flutter/engine#37865)

* 4101c363c [iOS] Change locale format for spell check (flutter/engine#38080)

* 2f5b67e4d [embedder] Ensure destruction called on present (flutter/engine#38078)

* 0bddc6045 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118)

* 6aa4ccd60 Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127)

* dbb5284f2 [Windows] Add more cursor plugin tests (flutter/engine#38112)

* 6e91204d9 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135)

* 3140ad924 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115)

* 84abf21d4 Remove autoninja. (flutter/engine#38136)

* 8a113d328 [embedder] Expose metal surface from test context (flutter/engine#38133)

* 1ef25b63f Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139)

* 748b3bc15 Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (#38127)" (flutter/engine#38137)

* b6daf3d06 [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141)

* 339d04baf [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126)

* 1fcbb9c11 [tools] Eliminate version on Obj-C docs (flutter/engine#38145)

* 71928b6a4 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146)

* 23ce8fdbc Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123)

* 366f8663b Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151)

* 447e7013e Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152)

* cd5d91bf9 Pylint testing/run_tests.py (flutter/engine#38016)

* aafac083b Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157)

* 799dc78e8 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134)

* 3aa3d2a8f Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147)

* 030950f30 Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Flutter Engine from 8d83b98c55b3 to 030950f3070c (29 revisions) (f…
76fa558 
…lutter#116802)

* bd8bcf956 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120)

* dec8b5221 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754)

* 5545ccf87 Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122)

* 80a15a419 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963)

* 5caef8585 Full implementation of text-input-test (flutter/engine#37986)

* 8f6036e58 Reland fix wrong VSYNC event (flutter/engine#37865)

* 4101c363c [iOS] Change locale format for spell check (flutter/engine#38080)

* 2f5b67e4d [embedder] Ensure destruction called on present (flutter/engine#38078)

* 0bddc6045 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118)

* 6aa4ccd60 Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127)

* dbb5284f2 [Windows] Add more cursor plugin tests (flutter/engine#38112)

* 6e91204d9 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135)

* 3140ad924 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115)

* 84abf21d4 Remove autoninja. (flutter/engine#38136)

* 8a113d328 [embedder] Expose metal surface from test context (flutter/engine#38133)

* 1ef25b63f Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139)

* 748b3bc15 Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (flutter#38127)" (flutter/engine#38137)

* b6daf3d06 [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141)

* 339d04baf [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126)

* 1fcbb9c11 [tools] Eliminate version on Obj-C docs (flutter/engine#38145)

* 71928b6a4 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146)

* 23ce8fdbc Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123)

* 366f8663b Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151)

* 447e7013e Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152)

* cd5d91bf9 Pylint testing/run_tests.py (flutter/engine#38016)

* aafac083b Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157)

* 799dc78e8 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134)

* 3aa3d2a8f Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147)

* 030950f30 Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@iskakaushik iskakaushik iskakaushik approved these changes

Assignees
No one assigned
Labels
needs tests platform-android platform-ios
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VSYNC trace event ending prematurely
4 participants
@fzyzcjy @chinmaygarde @iskakaushik @kenzieschmoll