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

target-bsnes: Do not set the window background colour. #126

Closed

Conversation

Screwtapello
Copy link
Contributor

The bsnes window is mostly covered by the viewport, which is supposed to be black. When the window is resized, the hiro Viewport widget repaints itself, according to whatever hiro backend is in use.

However, on some platforms that's not enough. Specifically on X11, when the user resizes a window the window manager, the X server, and the application can all have different ideas about the exact window size. The X11 protocol gives each window a "default background colour", to be used by the X server to fill the blanks after the window manager has resized the window, but before the application has been able to repaint it. Because the bsnes window is mostly black, bsnes sets its window background colour to black, so the flickering of X-server-drawn and application-drawn content will be less obvious.

Unfortunately, it gets more complex. GTK+2 was designed around the X11 model, and so it works basically the way bsnes expects. GTK+3 was designed around the more modern compositing model, which doesn't need that "default background colour" hack, and as a result when you set the window background colour on GTK+3. that background colour is used for everything. Even the menu-bar, which by default draws text in a very dark grey, so you can hardly read what it says.

Perhaps there is a way to both avoid flicker on GTK+2 and not break GTK+3, but until we find it, I would rather occasional flicker on GTK+2 than permanently broken GTK+3.

Fixes #108.

@Screwtapello
Copy link
Contributor Author

Kawa very kindly tested this branch on Win32, and discovered that if you resize the window enough without a game loaded, eventually it forgets to paint part of the viewport:

image

That's probably a deal-breaker. I assume the Viewport widget has an actual child-window associated with it (because it needs to be associated with a 3D rendering context), but the hiro Viewport widget itself does not have a .setBackgroundColor() method. Maybe individual hiro backends can be taught to set the background colour on the Viewport widget, without having to set the background for the entire window.

@ghisvail
Copy link
Contributor

Sure it's an anonying graphic glitch, but does it still appear once a game is loaded?

@ghost
Copy link

ghost commented Dec 22, 2020

the hiro Viewport widget itself does not have a .setBackgroundColor() method

Looking at the code it acts as though it's forcefully set to black always. I don't know if that's ideal but if it's meant for rendering OpenGL video I guess that makes sense.

That corner at the bottom right appears to not be the viewport though. bsnes uses some layout tricks to put a canvas widget to draw the program icon.

https://github.com/bsnes-emu/bsnes/blob/master/bsnes/target-bsnes/presentation/presentation.hpp#L127

The canvas and viewport classes have very different painting functions.

https://github.com/bsnes-emu/bsnes/blob/master/hiro/windows/widget/canvas.cpp#L92

https://github.com/bsnes-emu/bsnes/blob/master/hiro/windows/widget/viewport.cpp#L52

I don't know Win32 enough to try and "fix" this, or even if it can be fixed. Maybe ask @jchv?

As a test, maybe we could disable the bsnes icon on the window. I know it's blasphemy but it could be the easy fix here if viewport works and canvas doesn't.

@Kawa-oneechan
Copy link
Contributor

Kawa-oneechan commented Dec 22, 2020

Kawa very kindly tested this branch on Win32, and discovered that if you resize the window enough without a game loaded, eventually it forgets to paint part of the viewport:

Correction: it started like that. Although resizing was smooth as silk, that gray bit was a constant. But at least a smooth constant.

Sure it's an anonying graphic glitch, but does it still appear once a game is loaded?

No, the games look fine.

@Screwtapello
Copy link
Contributor Author

It turns out that the grey rectangle appears on GTK+2 and GTK+3 too - I didn't see it, because I happened to have "show falling snow" enabled rather than the emulator logo.

I wonder how that logo colouring is done.

@Screwtapello
Copy link
Contributor Author

OK, after a bit of digging, here's what I've found. The main bsnes window layout looks like this:

+------------------------+--------------+
| viewport               | iconSpacer   |
|                        |              |
|                        |              |
|                        |              |
|                        |              |
|                        |              |
|                        |              |
|                        +------------+-+
|                        | iconCanvas | |
|                        |            | |
|                        |            | |
+------------------------+------------+-+

viewport is a Viewport widget, which always paints itself black.

iconSpacer is a Canvas widget, configured to paint itself black.

iconCanvas is a Canvas widget, configured to paint itself itself with the bsnes icon. Or, nearly — the bsnes icon is transparent, but before it's given to the canvas, all the transparent bits are painted black, so it basically paints itself black.

Then where does the grey strip come from? iconSpacer is configured to be 144px wide. The bsnes icon is 128px wide, so iconCanvas is configured to be 128px wide, and therefore there's an unpainted 16×128px gap.

Of course, we can just set iconCanvas to be 144px wide, but then the icon is drawn centered in the canvas, leaving an 8px gap down each side — still not quite what we wanted.

@Kawa-oneechan
Copy link
Contributor

Kawa-oneechan commented Dec 24, 2020

Of course, we can just set iconCanvas to be 144px wide, but then the icon is drawn centered in the canvas, leaving an 8px gap down each side — still not quite what we wanted.

Obviously you set iconSpacer to be 128px wide.

If the assigned icon doesn't cover the entire allocated display area, we need to
fill the gaps somehow.

FIXME: Implement for Cocoa backend.
The bsnes window is mostly covered by the viewport, which is supposed to be
black. When the window is resized, the hiro Viewport widget repaints itself,
according to whatever hiro backend is in use.

However, on some platforms that's not enough. Specifically on X11, when the user
resizes a window the window manager, the X server, and the application can all
have different ideas about the exact window size. The X11 protocol gives each
window a "default background colour", to be used by the X server to fill the
blanks after the window manager has resized the window, but before the
application has been able to repaint it. Because the bsnes window is *mostly*
black, bsnes sets its window background colour to black, so the flickering of
X-server-drawn and application-drawn content will be less obvious.

Unfortunately, it gets more complex. GTK+2 was designed around the X11 model,
and so it works basically the way bsnes expects. GTK+3 was designed around the
more modern compositing model, which doesn't need that "default background
colour" hack, and as a result when you set the window background colour on
GTK+3. that background colour is used for *everything*. Even the menu-bar, which
by default draws text in a very dark grey, so you can hardly read what it says.

To fix this problem, we stop setting the background of the entire window to
black, and instead only set the background of the widgets in the middle of the
window. Specifically, the viewport widget is always black anyway, but when no
game is loaded bsnes has a Canvas widget that displays the bsnes icon.
Previously, a Canvas widget would show a background colour *or* an icon, but now
that an icon can have a padding color, we can use that to make sure the icon
background is black, regardless of the canvas size.

Fixes bsnes-emu#108.
@Screwtapello
Copy link
Contributor Author

Obviously you set iconSpacer to be 128px wide.

You'd think that, wouldn't you! But no, I decided to do the HARD THING.

I've modified the hiro Canvas::setIcon() API to take a padding colour (defaulting to "transparent") as well as an icon. The (complete) canvas widget is painted in the padding colour before the icon is composited onto it. That means we can make iconCanvas 144px wide, left-aligned (to preserve the current spacing), with black padding, and now the icon looks correct again.

I say I decided to do the HARD THING, but actually I wimped out in two respects:

  • GDI does not support filling a rectangle with a colour that includes an alpha channel. So, if the alpha is 0 (fully transparent) it just uses the parent's background colour as before, while if the alpha is anything else it draws the colour fully opaque.
  • I hav 0 idea about Cocoa, hopefully somebody who can build on macOS can help me out here.

@Screwtapello
Copy link
Contributor Author

Having slept on it, I am now less convinced of this approach. Given the clunkiness of GDI and who knows how difficult the Cocoa changes would be, the actual practical solution would just be to add another black Canvas widget to fill the 16×128px gap.

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.

hiro gtk3: Black menu bar
3 participants