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

layered base fixes & tests #3683

merged 6 commits into from Dec 29, 2018


None yet
3 participants
Copy link

hiiamboris commented Dec 26, 2018

Wow it got the tests passed ☺

OK. This is a Windows-related fix, mostly W7-related.
@qtxie @greggirwin and @PeterWAWood please review this one.

Basically this was meant to fix 3 things:

  1. text alignment on all your base face #2503 (comment)
  2. to-image inability to capture tricky layered W7 base faces #3465
  3. make size-text results somewhat relevant to the actual text displayed, with multi line support

Brief summary of changes and findings:

  • Pursuing (1) I had to drop GDI text rendering in favor of GDI+.
  • Pursuing (2) I had to also use GDI+ for background rendering, as older GDI parts are totally unaware of the alpha channel and thus FillRect's output is transparent from AlphaBlend's point of view. To capture the output of the layered bases I walk thru the target window's pane and blend every found base onto it.
  • You can read on the topic of WM_PRINT* messages. Fact is - they are never sent.
  • I made system/view/metrics/colors/text available for Red code. text and window are like yin and yang of the whole OS theme. Pointless to have one without the other.

Everything else is either minor refactoring or obvious minor fixes.

Then the second part.

View subsystem is already very complicated. For example, it's almost impossible to tell what function will draw the background or text just by following the code. It is in very bad need of refactoring. However we don't really have tests for it, so any attempt will likely ruin what's already working. Moreover I've watched it regress not a single time. Sooner or later we are bound to start testing it, so why not today? ☻

I've included 300+ tests for the changes this PR brings about, that capture the output and analyze if what was drawn is what was requested. Tested and working on live W7 and W8.

My observation of PrintWindow on W8 is that it captures the real visible state of the window, so:

  • if you close the window quickly, it won't capture a thing - this is the most annoying part, since window has to be displayed for about 1/2 second to be properly captured
  • there are some animation effects that make windows appear smoothly and because of that the white 255.255.255 window can be captured as 249.249.249 and then some
  • sometimes it only partly captures windows that are clipped by the screen edge, but I haven't observed a pattern

Of course all these effects can be successfully dealt with.

W10 volunteers are welcome to give it a try: build the console from this branch and run tests/source/view/ Report any misbehavior ;)

I decided to include this test into tests/run-all.r (which is used by AppVeyor), but not into the run-all.r from the root directory (which is described on the GH main page as the way to run the test suite), because:

  • I'd like any regressions to be caught timely and automatically
  • I wouldn't like to run this test with every change myself or impose that on anyone else, because it displays lots of windows and thus interferes with the PC usage for about a minute

Let me know if there is a better way. For the time being this makes run-all.r and tests/run-all.r diverge. A thing to keep in mind.

hiiamboris added some commits Dec 26, 2018

Copy link

qtxie left a comment

Looks quite good for me. 👍 Thank you very much! 💯

Show resolved Hide resolved modules/view/backends/windows/gui.reds Outdated
Show resolved Hide resolved modules/view/backends/windows/gui.reds Outdated
Show resolved Hide resolved modules/view/backends/windows/events.reds

hiiamboris added some commits Dec 27, 2018


This comment has been minimized.

Copy link

greggirwin commented Dec 29, 2018

I'll defer to @qtxie on this, and accept his review as enough.

@greggirwin greggirwin merged commit afa2dcd into red:master Dec 29, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed

@hiiamboris hiiamboris deleted the hiiamboris:layered branch Dec 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.