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

Improve clarity in Multiple Resolutions doc #9881

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaelgundlach
Copy link
Contributor

I tried to make this doc page clearer and remove bugs, because I find it hard to understand as is.

  • The resizing discussion mixed talking about resizing to fit a screen and resizing to fit a window. Because the animated gifs do a great job of demonstrating the Stretch Mode differences using resizing of windows, I changed most "screen" references to "window" references.

  • Replaced the word "display" with "screen" in most cases, as it's an unneeded synonym that made me wonder if there was a difference.

  • Corrected several instances of referring to the viewport width and height settings as window width and height, or base window width and base window height.

  • Standardized referring to "base size" in most places instead of sometimes referring to "viewport size" or to "base window size".

  • Fleshed out the description of Canvas Item vs Viewport stretch mode. Maybe I made it worse, but I had to go onto reddit to find a clearer description than what existed. What clicked for me was a reddit summary saying "Canvas Items first scales the images, and then renders them. Viewport renders the images, and then scales them." I was then able to (hopefully!) clarify the mode descriptions.

  • Replaced Keep Height description with basically "like Keep Width but for height", rather than making the user check the copy-and-modified description vs Keep Width's for differences.

  • Asserted that Stretch Mode=Viewport completely avoids scaling artifacts when Stretch Scale Mode is set to Integer. If I'm wrong, please let me know. It's still not clear to me why the Stretch Mode=Viewport example animated gif never looks fuzzy, assuming the author recorded it using Stretch Scale Mode=Fractional, since the Stretch Scale Mode section seems to imply that random resizing of small resolution sprites should cause artifacts unless Stretch Scale Mode=Integer.

  • Applied style guide to some long lines.

I may have introduced bugs due to not perfectly understanding the subject matter, and am happy to revise further if you explain what I got wrong.

I tried to make this doc page clearer and remove bugs, because I find it
hard to understand as is.

- The resizing discussion mixed talking about resizing to fit a screen
  and resizing to fit a window.  Because the animated gifs do a great
  job of demonstrating the Stretch Mode differences using resizing of
  windows, I changed most "screen" references to "window" references.

- Replaced the word "display" with "screen" in most cases, as it's an
  unneeded synonym that made me wonder if there was a difference.

- Corrected several instances of referring to the viewport width and
  height settings as window width and height, or base window width and
  base window height.

- Standardized referring to "base size" in most places instead of
  sometimes referring to "viewport size" or to "base window size".

- Fleshed out the description of Canvas Item vs Viewport stretch mode.
  Maybe I made it worse, but I had to go onto reddit to find a clearer
  description than what existed.  What clicked for me was a reddit summary
  saying "Canvas Items first scales the images, and then renders them.
  Viewport renders the images, and then scales them." I was then able to
  (hopefully!) clarify the mode descriptions.

- Replaced Keep Height description with basically "like Keep Width but
  for height", rather than making the user check the copy-and-modified
  description vs Keep Width's for differences.

- Asserted that Stretch Mode=Viewport completely avoids scaling
  artifacts when Stretch Scale Mode is set to Integer.  If I'm wrong,
  please let me know.  It's still not clear to me why the Stretch
  Mode=Viewport example animated gif never looks fuzzy, assuming the
  author recorded it using Stretch Scale Mode=Fractional, since the
  Stretch Scale Mode section seems to imply that random resizing of small
  resolution sprites should cause artifacts unless Stretch Scale
  Mode=Integer.

- Applied style guide to some long lines.

I may have introduced bugs due to not perfectly understanding the
subject matter, and am happy to revise further if you explain what I got
wrong.
@skyace65 skyace65 added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Sep 5, 2024
@michaelgundlach
Copy link
Contributor Author

If this is too large a change please let me know and I can break it up. Or, if it made it too hard to review because I fixed line spacing, I can undo that.

@AThousandShips AThousandShips changed the title Reduce imprecision in Multiple Resolutions doc Improve clarity in Multiple Resolutions doc Sep 18, 2024
@tetrapod00
Copy link
Contributor

tetrapod00 commented Nov 24, 2024

Hey, sorry for the long wait on any feedback. I skimmed this and it generally looks good, and I've been meaning to review it more deeply.

If this is too large a change please let me know and I can break it up.

This is more for future reference, but:

Splitting things can definitely make things easier to review, if you can split out the easily verifiable changes from the changes that require more knowledge to verify (and therefore require a reviewer with niche knowledge who may not be available). In this case, the page seems to contain a mixture of both intermixed, so I'm not sure splitting things would be easy or help.

Splitting out new content from changes to existing content can also be useful.

Splitting merely into sections of the page might not help, if each section still requires a specific area reviewer.

too hard to review because I fixed line spacing

Fixing lines to wrap at 80-100 characters is good practice. It does make things slightly harder to review, but that's unavoidable. It's not a factor in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants