Skip to content

Fix non-native full screen misc crash, background color, and transparency issues #1521

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

Merged
merged 3 commits into from
Jan 7, 2025

Conversation

ychin
Copy link
Member

@ychin ychin commented Jan 5, 2025

Fixed misc bugs:

  • Previously, setting a colorscheme while in non-native full screen would override the background color specified in 'fuoptions' (default is black). It also wouldn't update the specified color if it's specified to a highlight group (e.g. set fuopts=background:Normal).

  • Transparency and background colors and setting colorscheme also didn't work well togehter. Changing a background color while in fullscreen would reset transparency back to 0.

Also, 'fuoptions' can now specify background color in 6-digit hex number now, in addition to the old 8-digit format. Also update docs to elaborate that when using 8-digit, the alpha component is ignored (since we use the 'transparency' setting instead).

Add new tests to test 'fuoptions' work.


Also include the following commits:

Fix non-native full screen bad interaction with window delegate

This issue was detected when adding new tests for non-native full screen. When setting 'fuoptions' to empty and going full screen, the code would fail but also crash, due to a number of issues:

  1. enterFullScreen's setting of presentationOptions somehow triggers a window resize on the normal window, which leads to windowDidResize delegate being called. This is a degenerate situation as the window controller thinks we are already in full screen even though we haven't finished setting up yet, and in particular nonFuVimViewSize is still 0. This leads to the desired desired frame size being set incorrectly to 0.
  2. constrainRows:columns:toSize does not sanity check the results when we have such degenerate small sizes, and end up calculating a desired rows to -1, which makes no sense. This leads to various wrong calculations.
  3. MMCoreTextView loops through the grid using a size_t even though grid.rows is an int. This is a poor code practice in general and results in a crash as the loop comparison cast the -1 to size_t and the loop didn't correctly terminate.

Fix 1 (the root cause) by making sure we detact the window delegate immediately when entering full screen to prevent any stray window messages from causing issues. Also safeguard windowDidResize so it only handles the full screen path if we have fullScreenEnabled set.

For 2 and 3, add the sanity checks and also fix the size_t to use int when looping.

For some reason this issue only showed up in CI but not in local testing, probably due to different screen environments.

Add tests for full screen code

Also, add some additional tests for full screen mode in general.

Basic tests to validate different full screen functionalities. Help prevents regressions such as #1515 where full screen simply stops working.

Not an exhaustive test for now. Some functionalities (e.g. multi-monitor, MacBook notch) are hard to mock up and validate in tests. Some other functionalities like restoring window size, delayed full screen on startup (when setting it from gvimrc) will be added later.

@ychin ychin added this to the Release 181 milestone Jan 5, 2025
Basic tests to validate different full screen functionalities. Help
prevents regressions such as macvim-dev#1515 where full screen simply stops
working.

Not an exhaustive test for now. Some functionalities (e.g.
multi-monitor, MacBook notch) are hard to mock up and validate in tests.
Some other functionalities like restoring window size, delayed full
screen on startup (when setting it from gvimrc) will be added later.
@ychin ychin force-pushed the fullscreen-tests-fuopt-fixes branch from c9c91c9 to 53dc316 Compare January 7, 2025 03:25
ychin added 2 commits January 6, 2025 19:26
This issue was detected when adding new tests for non-native full
screen. When setting 'fuoptions' to empty and going full screen, the
code would fail but also crash, due to a number of issues:

1. `enterFullScreen`'s setting of presentationOptions somehow triggers a
   window resize on the normal window, which leads to `windowDidResize`
   delegate being called. This is a degenerate situation as the window
   controller thinks we are already in full screen even though we
   haven't finished setting up yet, and in particular `nonFuVimViewSize`
   is still 0. This leads to the desired desired frame size being set
   incorrectly to 0.
2. `constrainRows:columns:toSize` does not sanity check the results when
   we have such degenerate small sizes, and end up calculating a desired
   rows to -1, which makes no sense. This leads to various wrong
   calculations.
3. MMCoreTextView loops through the grid using a size_t even though
   grid.rows is an int. This is a poor code practice in general and
   results in a crash as the loop comparison cast the -1 to size_t and
   the loop didn't correctly terminate.

Fix 1 (the root cause) by making sure we detact the window delegate
immediately when entering full screen to prevent any stray window
messages from causing issues. Also safeguard `windowDidResize` so it
only handles the full screen path if we have `fullScreenEnabled` set.

For 2 and 3, add the sanity checks and also fix the size_t to use int
when looping.

For some reason this issue only showed up in CI but not in local
testing, probably due to different screen environments.
Fixed misc bugs:

- Previously, setting a colorscheme while in non-native full screen would
override the background color specified in 'fuoptions' (default is
black). It also wouldn't update the specified color if it's specified to
a highlight group (e.g. `set fuopts=background:Normal`).

- Transparency and background colors and setting colorscheme also didn't
work well togehter. Changing a background color while in fullscreen
would reset transparency back to 0.

Also, 'fuoptions' can now specify background color in 6-digit hex number
now, in addition to the old 8-digit format. Also update docs to
elaborate that when using 8-digit, the alpha component is ignored (since
we use the 'transparency' setting instead).

Add new tests to test 'fuoptions' work.
@ychin ychin force-pushed the fullscreen-tests-fuopt-fixes branch from 53dc316 to 762a8c8 Compare January 7, 2025 04:17
@ychin ychin merged commit c93f9c0 into macvim-dev:master Jan 7, 2025
2 of 4 checks passed
@ychin ychin deleted the fullscreen-tests-fuopt-fixes branch January 7, 2025 04:51
@ychin ychin changed the title Fix non-native full screen misc background color and transparency issues Fix non-native full screen misc crash, background color, and transparency issues Jan 7, 2025
@ychin
Copy link
Member Author

ychin commented Jan 7, 2025

This actually fixes a MacVim free that could be easily reproduced if you have the following in the gvimrc:

set fuopt=
set fullscreen

The delayed full screen logic before would delay setting fullScreenEnabled, but it would still initialize fullScreenWindow. The new code checks for that before calculating the desired size in the windowDidResize callback which guards against this situation correctly.

ychin added a commit to ychin/macvim that referenced this pull request Jan 8, 2025
Add test suite utility functions to set/restore defaults, adding/tearing
down a new Vim window, making temp files, wait for full screen
transition. This helps simplify per-test code and prevent mistakes.

Add new tests for delayed full screen on startup. This happens when a
vimrc/gvimrc sets 'fullscreen' on startup and MacVim has to delay
entering full screen until the window has been presented. This
incidentally regression tests a bug (fixed in macvim-dev#1521) where simply having
'set fuopt= fullscreen' in a gvimrc would cause MacVim to crash on
startup due to bad interaction with window resize messages.
ychin added a commit to ychin/macvim that referenced this pull request Jan 8, 2025
Add test suite utility functions to set/restore defaults, adding/tearing
down a new Vim window, making temp files, wait for full screen
transition. This helps simplify per-test code and prevent mistakes.

Add new tests for delayed full screen on startup. This happens when a
vimrc/gvimrc sets 'fullscreen' on startup and MacVim has to delay
entering full screen until the window has been presented. This
incidentally regression tests a bug (fixed in macvim-dev#1521) where simply having
'set fuopt= fullscreen' in a gvimrc would cause MacVim to crash on
startup due to bad interaction with window resize messages.
ychin added a commit to ychin/macvim that referenced this pull request Jan 8, 2025
Add test suite utility functions to set/restore defaults, adding/tearing
down a new Vim window, making temp files, wait for full screen
transition. This helps simplify per-test code and prevent mistakes.

Add new tests for delayed full screen on startup. This happens when a
vimrc/gvimrc sets 'fullscreen' on startup and MacVim has to delay
entering full screen until the window has been presented. This
incidentally regression tests a bug (fixed in macvim-dev#1521) where simply having
'set fuopt= fullscreen' in a gvimrc would cause MacVim to crash on
startup due to bad interaction with window resize messages.
ychin added a commit to ychin/macvim that referenced this pull request Jan 9, 2025
Add test suite utility functions to set/restore defaults, adding/tearing
down a new Vim window, making temp files, wait for full screen
transition. This helps simplify per-test code and prevent mistakes.

Add new tests for delayed full screen on startup. This happens when a
vimrc/gvimrc sets 'fullscreen' on startup and MacVim has to delay
entering full screen until the window has been presented. This
incidentally regression tests a bug (fixed in macvim-dev#1521) where simply having
'set fuopt= fullscreen' in a gvimrc would cause MacVim to crash on
startup due to bad interaction with window resize messages.
ychin added a commit that referenced this pull request Feb 20, 2025
Updated to Vim 9.1.1128

This update contains a completely new GUI tabs implementation by @sfsam!
It also contains lots of small fixes for window resizing and full screen
mode that aims to make using MacVim feel rock solid and stable.

Defaults Change
====================

New settings defaults related to window sizing #1528:

- "Smoothly resizes window" is now on by default. MacVim's window will
  now resize smoothly instead of snapped to the size of the character
  grid.
- Vim's `guioption` now has `k` set by default (`:h go-k`). This
  prevents MacVim's window size from changing unnecessarily when
  showing/hiding tabs or changing font size.

These should align MacVim better with how other apps work and integrate
better with OS window management, including macOS 15 Sequoia's window
tiling feature.

Features
====================

Tabs
--------------------

MacVim has a new tabs implementation! The old version (PSMTabBarControl)
is not maintained and lacks features such as overflowing tabs and
customizable colors. The new tabs will overflow horizontally and are
scrollable. They also animate when tabs are closed or moved, respect
system settings such as right-to-left locales and high-contrast modes,
and are designed to fit within the currently selected Vim colors.

There are a few ways to customize the colors of the new tabs, under the
"Appearance" settings pane. MacVim defaults to an "Automatic colors"
mode which tries to pick sensible colors automatically based on the
current foreground/background colors. However, you can also configure it
to simply use the tab colors specified by the Vim color scheme (some
color schemes will work better than others depending on their choice of
colors). Another new option is "Use tabs background color" which when
combined with "Transparent title bar" allows the title bar and tabs to
look like a single cohesive whole.

Relevant work:

- #1120 (by @sfsam)
- Also: #1535 / #1536 / #1537 / #1538 / #1539 / #1557 / #1558 / #1560

New Vim features
--------------------

- new bundled color scheme:
    - unokai (vim/vim#16443)
- new bundled optional plugins (use `packadd` to enable them):
    - helptoc: Use `:HelpToc` to show an interactive table of contents
      for Vim help, man pages, Markdown files, and terminal.
      vim/vim#10446
- new options:
    - `set diffopt+=linematch:{n}`. Matches lines better when in diff
      mode. v9.1.1099
    - `findfunc`. Customizes `:find` and other commands. v9.1.0831
    - `set completeopt+=preinsert`. Preview inserted text in completion.
      v9.1.1056
    - `messagesopt`. Allows customizing hit-enter behavior. v9.1.0908
- new functions:
    - `getcellpixels()`. Query the pixel size of a character cell in the
      grid. v9.1.0854 / #1554 / #1555
- Vim tutor has a new interactive plugin (v9.1.0836). There is also now
  a chapter 2 (vim/vim#5729).

Misc New Settings
--------------------

- "Open untitled window" (General) has a new option to only open on
  MacVim re-activation. #1509
- "Show document icon at title bar" (Appearance). Previously MacVim
  implicitly hid the document icon when using transparent title bar.
  This is now customizable. #1510

General
====================

- The MacVim dmg installer has a new design. Courtesy of @jasonlong.
  #1540 #1545
- Legacy builds (macOS 10.9 - 10.12) are no longer built by GitHub
  hosted runners, due to GitHub's deprecation of old runners. They are
  now built by a custom self-hosted VM instead. In the future we hope to
  set up reproducible builds (#1506) so it will not matter who's
  building the app as it would be verifiable. #1559
- "Nightly" build: We now build a dmg installer for every commit. This
  allows for trying out the latest developmental version of MacVim, but
  note that the app will not be signed / notarized, and it will not be
  as polished as official release/pre-release builds. See
  [wiki](https://github.com/macvim-dev/macvim/wiki/Installing) for
  instructions. #1532

Fixes
====================

Apple "Intelligence" Writing Tools
--------------------

macOS 15 Sequoia's Apple "Intelligence" Writing Tools should work
correctly with MacVim now. To use it, select some text, right click to
show menu, and then select the "Writing Tools" sub-menu. As part of this
fix, the integration with the "Services" menu now works more reliably as
well. You can select texts in blockwise visual mode and select a service
and MacVim will try to place the new texts back to the blockwise
selection if possible. #1552

Window resizing and full screen
--------------------

- Flicker begone: Changing font size, showing/hiding tabs or scroll
  bars, or entering non-native full screen should no longer cause MacVim
  to flicker. Previously there could be a momentary but
  distracting/annoying stale image that flashes briefly. #1547 #1549
- Fixed issue where resizing MacVim window would occasionally cause Vim
  to be stuck in a stale wrong size. #1518
- Non-native full screen now supports `blurradius` option. #1546
- Fixed window size not always restoring correctly when exiting full
  screen. Non-native full screen also works more reliably in
  multi-monitor setup. #1525
- Fixed non-native full screen mode when using an external monitor with
  a MacBook with a notch, and having the "Show menu bar in non-native
  mode" option set. Previously MacVim would sometimes miscalculate the
  menu bar height in the second screen. #1548
- Fixed misc issues with non-native full screen's interaction with
  `fuoptions` and also the `transparency` setting, and rare crash. #1521

Other Fixes
--------------------

- Fixed issue where changing font size (using Cmd =/-) with guifont set
  to "-monospace-" would result in guifont being changed to a confusing
  name like ".AppleSystemUIFontMonospaced-Regular". #1544
- "MacVim Website" menu item now goes to the updated URL. #1524
- What's New page now allows changing font size (using Cmd =/-), and
  showing table of contents. #1561 #1562
- Dark mode documentation is now a bit clearer on `v:os_appearance`.
  #1511
- Using dictionary look up on selected texts (by right clicking and then
  selecting "Look Up" in the pop-up menu) is now more resilient as it
  uses Vim's native `getregion()` to determine the selected texts. #1508

Scripting
====================

- Scripting languages versions:
    - Ruby is now built against 3.4, up from 3.3.
    - Perl is now built against 5.34, up from 5.30.

Compatibility
====================

Requires macOS 10.9 or above. (10.9 - 10.12 requires downloading a
separate legacy build)

Script interfaces have compatibility with these versions:

- Lua 5.4
- Perl 5.34
- Python2 2.7
- Python3 3.9 or above
- Ruby 3.4
ychin added a commit that referenced this pull request Feb 21, 2025
Updated to Vim 9.1.1128

This update contains a completely new GUI tabs implementation by @sfsam!
It also contains lots of small fixes for window resizing and full screen
mode that aims to make using MacVim feel rock solid and stable.

Defaults Change
====================

New settings defaults related to window sizing #1528:

- "Smoothly resizes window" is now on by default. MacVim's window will
  now resize smoothly instead of snapped to the size of the character
  grid.
- Vim's `guioption` now has `k` set by default (`:h go-k`). This
  prevents MacVim's window size from changing unnecessarily when
  showing/hiding tabs or changing font size.

These should align MacVim better with how other apps work and integrate
better with OS window management, including macOS 15 Sequoia's window
tiling feature.

Features
====================

Tabs
--------------------

MacVim has a new tabs implementation! The old version (PSMTabBarControl)
is not maintained and lacks features such as overflowing tabs and
customizable colors. The new tabs will overflow horizontally and are
scrollable. They also animate when tabs are closed or moved, respect
system settings such as right-to-left locales and high-contrast modes,
and are designed to fit within the currently selected Vim colors.

There are a few ways to customize the colors of the new tabs, under the
"Appearance" settings pane. MacVim defaults to an "Automatic colors"
mode which tries to pick sensible colors automatically based on the
current foreground/background colors. However, you can also configure it
to simply use the tab colors specified by the Vim color scheme (some
color schemes will work better than others depending on their choice of
colors). Another new option is "Use tabs background color" which when
combined with "Transparent title bar" allows the title bar and tabs to
look like a single cohesive whole.

Relevant work:

- #1120 (by @sfsam)
- Also: #1535 / #1536 / #1537 / #1538 / #1539 / #1557 / #1558 / #1560

New Vim features
--------------------

- new bundled color scheme:
    - unokai (vim/vim#16443)
- new bundled optional plugins (use `packadd` to enable them):
    - helptoc: Use `:HelpToc` to show an interactive table of contents
      for Vim help, man pages, Markdown files, and terminal.
      vim/vim#10446
- new options:
    - `set diffopt+=linematch:{n}`. Matches lines better when in diff
      mode. v9.1.1099
    - `findfunc`. Customizes `:find` and other commands. v9.1.0831
    - `set completeopt+=preinsert`. Preview inserted text in completion.
      v9.1.1056
    - `messagesopt`. Allows customizing hit-enter behavior. v9.1.0908
- new functions:
    - `getcellpixels()`. Query the pixel size of a character cell in the
      grid. v9.1.0854 / #1554 / #1555
- Vim tutor has a new interactive plugin (v9.1.0836). There is also now
  a chapter 2 (vim/vim#5729).

Misc New Settings
--------------------

- "Open untitled window" (General) has a new option to only open on
  MacVim re-activation. #1509
- "Show document icon at title bar" (Appearance). Previously MacVim
  implicitly hid the document icon when using transparent title bar.
  This is now customizable. #1510

General
====================

- The MacVim dmg installer has a new design. Courtesy of @jasonlong.
  #1540 #1545
- Legacy builds (macOS 10.9 - 10.12) are no longer built by GitHub
  hosted runners, due to GitHub's deprecation of old runners. They are
  now built by a custom self-hosted VM instead. In the future we hope to
  set up reproducible builds (#1506) so it will not matter who's
  building the app as it would be verifiable. #1559
- "Nightly" build: We now build a dmg installer for every commit. This
  allows for trying out the latest developmental version of MacVim, but
  note that the app will not be signed / notarized, and it will not be
  as polished as official release/pre-release builds. See
  [wiki](https://github.com/macvim-dev/macvim/wiki/Installing) for
  instructions. #1532

Fixes
====================

Apple "Intelligence" Writing Tools
--------------------

macOS 15 Sequoia's Apple "Intelligence" Writing Tools should work
correctly with MacVim now. To use it, select some text, right click to
show menu, and then select the "Writing Tools" sub-menu. As part of this
fix, the integration with the "Services" menu now works more reliably as
well. You can select texts in blockwise visual mode and select a service
and MacVim will try to place the new texts back to the blockwise
selection if possible. #1552

Window resizing and full screen
--------------------

- Flicker begone: Changing font size, showing/hiding tabs or scroll
  bars, or entering non-native full screen should no longer cause MacVim
  to flicker. Previously there could be a momentary but
  distracting/annoying stale image that flashes briefly. #1547 #1549
- Fixed issue where resizing MacVim window would occasionally cause Vim
  to be stuck in a stale wrong size. #1518
- Non-native full screen now supports `blurradius` option. #1546
- Fixed window size not always restoring correctly when exiting full
  screen. Non-native full screen also works more reliably in
  multi-monitor setup. #1525
- Fixed non-native full screen mode when using an external monitor with
  a MacBook with a notch, and having the "Show menu bar in non-native
  mode" option set. Previously MacVim would sometimes miscalculate the
  menu bar height in the second screen. #1548
- Fixed misc issues with non-native full screen's interaction with
  `fuoptions` and also the `transparency` setting, and rare crash. #1521

Other Fixes
--------------------

- Fixed issue where changing font size (using Cmd =/-) with guifont set
  to "-monospace-" would result in guifont being changed to a confusing
  name like ".AppleSystemUIFontMonospaced-Regular". #1544
- "MacVim Website" menu item now goes to the updated URL. #1524
- What's New page now allows changing font size (using Cmd =/-), and
  showing table of contents. #1561 #1562
- Dark mode documentation is now a bit clearer on `v:os_appearance`.
  #1511
- Using dictionary look up on selected texts (by right clicking and then
  selecting "Look Up" in the pop-up menu) is now more resilient as it
  uses Vim's native `getregion()` to determine the selected texts. #1508

Scripting
====================

- Scripting languages versions:
    - Ruby is now built against 3.4, up from 3.3.
    - Perl is now built against 5.34, up from 5.30.

Compatibility
====================

Requires macOS 10.9 or above. (10.9 - 10.12 requires downloading a
separate legacy build)

Script interfaces have compatibility with these versions:

- Lua 5.4
- Perl 5.34
- Python2 2.7
- Python3 3.9 or above
- Ruby 3.4
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