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

Conversation

@iskakaushik
Copy link
Contributor

Issue: flutter/flutter#74797

Consider the following scenario:

  1. Window is created.
  • resize_target_width_ and resize_target_height_ are zero.
  • onscreen_surface_ dims are window dims.
  1. Window is immediately minimized.
  • resize target dims still zero.
  • onscreen_surface_ dims are window dims.
  1. Window is then restored.
  • Surface will not be updated as the window dims are still
    onscreen_surface_ dims.
  • Hence we need to compare the target dims with surface dims rather
    than resize_target dims as they will continue to be zero.

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

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.

@iskakaushik
Copy link
Contributor Author

iskakaushik commented Feb 5, 2021

@stuartmorgan I'm looking into how feasible writing a test for this is. I also did not encounter this before, as, in my testing I was resizing before minimizing and that ended up not hitting this code-path :-(

Consider the following scenario:

1. Window is created.
  - resize_target_width_ and resize_target_height_ are zero.
  - onscreen_surface_ dims are window dims.
2. Window is immediately minimized.
  - resize target dims still zero.
  - onscreen_surface_ dims are window dims.
3. Window is then restored.
  - Surface will not be updated as the window dims are still
  onscreen_surface_ dims.
  - Hence we need to compare the target dims with surface dims rather
  than resize_target dims as they will continue to be zero.
@iskakaushik iskakaushik force-pushed the cur_width_also_non_zero branch from 5500d13 to a4e80fd Compare February 5, 2021 16:39
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM.

I can't remember if we have any Windows tests currently that actually run the engine (vs stubbing it out), like we do for at least Linux. If we do, it seems like we could send the right WM_* messages in to see if it hangs or not.

@iskakaushik iskakaushik merged commit e4e3eb6 into flutter:master Feb 5, 2021
@iskakaushik iskakaushik deleted the cur_width_also_non_zero branch February 5, 2021 18:17
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 9, 2021
christopherfujino added a commit that referenced this pull request Feb 10, 2021
…24306)

* Update Dart SDK to 2.12.0-259.9.beta
* Fixed plumbing of the spawning isolate for Shell::Spawn. (#24112)
* [canvaskit] update CSS size of the canvas when device-pixel ratio changes (#24160)
* [web] Fix text selection from right to left (#24214)
* [windows] Surface will not update when restoring (#24236)

Co-authored-by: gaaclarke <[email protected]>
Co-authored-by: Yegor <[email protected]>
Co-authored-by: Mouad Debbar <[email protected]>
Co-authored-by: Kaushik Iska <[email protected]>
hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants