-
Notifications
You must be signed in to change notification settings - Fork 112
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge remote-tracking branch 'remotes/origin/feature/bezels'
- Loading branch information
Showing
50 changed files
with
4,816 additions
and
3,802 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,214 @@ | ||
| //============================================================================ | ||
| // | ||
| // SSSS tt lll lll | ||
| // SS SS tt ll ll | ||
| // SS tttttt eeee ll ll aaaa | ||
| // SSSS tt ee ee ll ll aa | ||
| // SS tt eeeeee ll ll aaaaa -- "An Atari 2600 VCS Emulator" | ||
| // SS SS tt ee ll ll aa aa | ||
| // SSSS ttt eeeee llll llll aaaaa | ||
| // | ||
| // Copyright (c) 1995-2023 by Bradford W. Mott, Stephen Anthony | ||
| // and the Stella Team | ||
| // | ||
| // See the file "License.txt" for information on usage and redistribution of | ||
| // this file, and for a DISCLAIMER OF ALL WARRANTIES. | ||
| //============================================================================ | ||
|
|
||
| #include <cmath> | ||
|
|
||
| #include "OSystem.hxx" | ||
| #include "Console.hxx" | ||
| #include "EventHandler.hxx" | ||
| #include "FBSurface.hxx" | ||
| #include "PNGLibrary.hxx" | ||
|
|
||
| #include "Bezel.hxx" | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| Bezel::Bezel(OSystem& osystem) | ||
| : myOSystem{osystem}, | ||
| myFB{osystem.frameBuffer()} | ||
| { | ||
| } | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| const string Bezel::getName(int& index) const | ||
| { | ||
| if(++index == 1) | ||
| return myOSystem.console().properties().get(PropType::Bezel_Name); | ||
|
|
||
| // Try to generate bezel name from cart name | ||
| const string& cartName = myOSystem.console().properties().get(PropType::Cart_Name); | ||
| size_t pos = cartName.find_first_of("("); | ||
| if(pos == std::string::npos) | ||
| pos = cartName.length() + 1; | ||
| if(index < 10 && pos != std::string::npos && pos > 0) | ||
| { | ||
| // The following suffixes are from "The Official No-Intro Convention", | ||
| // covering all used combinations by "The Bezel Project" (except single ones) | ||
| // (Unl) = unlicensed (Homebrews) | ||
| const std::array<string, 8> suffixes = { | ||
| " (USA)", " (USA) (Proto)", " (USA) (Unl)", " (USA) (Hack)", | ||
| " (Europe)", " (Germany)", " (France) (Unl)", " (Australia)" | ||
| }; | ||
| return cartName.substr(0, pos - 1) + suffixes[index - 2]; | ||
| } | ||
|
|
||
| if(index == 10) | ||
| { | ||
| index = -1; | ||
| return "default"; | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| uInt32 Bezel::borderSize(uInt32 x, uInt32 y, uInt32 size, Int32 step) const | ||
| { | ||
| uInt32 *pixels{nullptr}, pitch; | ||
| uInt32 i; | ||
|
|
||
| mySurface->basePtr(pixels, pitch); | ||
| pixels += x + y * pitch; | ||
|
|
||
| for(i = 0; i < size; ++i, pixels += step) | ||
| { | ||
| uInt8 r, g, b, a; | ||
|
|
||
| myFB.getRGBA(*pixels, &r, &g, &b, &a); | ||
| if(a < 255) | ||
| return i; | ||
| } | ||
| return size - 1; | ||
| } | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| bool Bezel::load() | ||
| { | ||
| bool isValid = false; | ||
|
|
||
| #ifdef IMAGE_SUPPORT | ||
| const bool isShown = myOSystem.eventHandler().inTIAMode() && | ||
| myOSystem.settings().getBool("bezel.show") && | ||
| (myFB.fullScreen() || myOSystem.settings().getBool("bezel.windowed")); | ||
|
|
||
| if(mySurface) | ||
| myFB.deallocateSurface(mySurface); | ||
| mySurface = nullptr; | ||
|
|
||
| if(isShown) | ||
| { | ||
| mySurface = myFB.allocateSurface(1, 1); // dummy size | ||
| try | ||
| { | ||
| const string& path = myOSystem.bezelDir().getPath(); | ||
| string imageName; | ||
| VariantList metaData; | ||
| int index = 0; | ||
|
|
||
| do | ||
| { | ||
| const string& name = getName(index); | ||
| if(name != EmptyString) | ||
| { | ||
| // Note: JPG does not support transparency | ||
| imageName = path + name + ".png"; | ||
| FSNode node(imageName); | ||
| if(node.exists()) | ||
| { | ||
| isValid = true; | ||
| myOSystem.png().loadImage(imageName, *mySurface, metaData); | ||
| break; | ||
| } | ||
| } | ||
| } while(index != -1); | ||
| } | ||
| catch(const runtime_error&) | ||
| { | ||
| isValid = false; | ||
| } | ||
| } | ||
| #endif | ||
| if(isValid) | ||
| { | ||
| const Settings& settings = myOSystem.settings(); | ||
| const Int32 w = mySurface->width(); | ||
| const Int32 h = mySurface->height(); | ||
| uInt32 top, bottom, left, right; | ||
|
|
||
| if(settings.getBool("bezel.win.auto")) | ||
| { | ||
| // Determine transparent window inside bezel image | ||
| uInt32 xCenter, yCenter; | ||
|
|
||
| xCenter = w >> 1; | ||
| top = borderSize(xCenter, 0, h, w); | ||
| bottom = h - 1 - borderSize(xCenter, h - 1, h, -w); | ||
| yCenter = (bottom + top) >> 1; | ||
| left = borderSize(0, yCenter, w, 1); | ||
| right = w - 1 - borderSize(w - 1, yCenter, w, -1); | ||
| } | ||
| else | ||
| { | ||
| // BP: 13, 13, 0, 0% | ||
| // HY: 12, 12, 0, 0% | ||
| // P1: 25, 25, 11, 22% | ||
| // P2: 23, 23, 7, 20% | ||
| left = std::min(w - 1, static_cast<Int32>(w * settings.getInt("bezel.win.left") / 100. + .5)); | ||
| right = w - 1 - std::min(w - 1, static_cast<Int32>(w * settings.getInt("bezel.win.right") / 100. + .5)); | ||
| top = std::min(h - 1, static_cast<Int32>(h * settings.getInt("bezel.win.top") / 100. + .5)); | ||
| bottom = h - 1 - std::min(h - 1, static_cast<Int32>(h * settings.getInt("bezel.win.bottom") / 100. + .5)); | ||
| } | ||
|
|
||
| //cerr << (int)(right - left + 1) << " x " << (int)(bottom - top + 1) << " = " | ||
| // << double((int)(right - left + 1)) / double((int)(bottom - top + 1)); | ||
|
|
||
| // Disable bezel is no transparent window was found | ||
| if (left < right && top < bottom) | ||
| myInfo = Info(Common::Size(w, h), Common::Rect(left, top, right, bottom)); | ||
| else | ||
| myInfo = Info(); | ||
| } | ||
| else | ||
| myInfo = Info(); | ||
|
|
||
| return isValid; | ||
| } | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| void Bezel::apply() | ||
| { | ||
| if(isShown()) | ||
| { | ||
| const uInt32 bezelW = | ||
| std::min(myFB.screenSize().w, | ||
| static_cast<uInt32>(std::round(myFB.imageRect().w() * myInfo.ratioW()))); | ||
| const uInt32 bezelH = | ||
| std::min(myFB.screenSize().h, | ||
| static_cast<uInt32>(std::round(myFB.imageRect().h() * myInfo.ratioH()))); | ||
|
|
||
| // Position and scale bezel | ||
| mySurface->setDstSize(bezelW, bezelH); | ||
| mySurface->setDstPos((myFB.screenSize().w - bezelW) / 2, // center | ||
| (myFB.screenSize().h - bezelH) / 2); | ||
| mySurface->setScalingInterpolation(ScalingInterpolation::sharp); | ||
| // Note: Variable bezel window positions are handled in VideoModeHandler::Mode | ||
|
|
||
| // Enable blending to allow overlaying the bezel over the TIA output | ||
| mySurface->attributes().blending = true; | ||
| mySurface->attributes().blendalpha = 100; | ||
| mySurface->applyAttributes(); | ||
| mySurface->setVisible(true); | ||
| } | ||
| else | ||
| if(mySurface) | ||
| mySurface->setVisible(false); | ||
| } | ||
|
|
||
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| void Bezel::render() | ||
| { | ||
| if(mySurface) | ||
| mySurface->render(); | ||
| } |
Oops, something went wrong.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thrust26, bug report time 😄
In Linux, there is a flashing of the TIA output when bezels are enabled. It happens in windowed and fullscreen modes. If I press Pause, then it's clear that one frame is black and another is rendered. That's what's causing it to appear flashing. Linux is using OpenGL.
In Windows, it worked fine in windowed mode at first, until I resized to a certain size. Pick a size that the normal (non-bezel) TIA image would fit on the desktop, but would be too large to fit the bezel. Basically a size that is very near the desktop resolution. When turning on bezels at that resolution, you get the same flashing as in Linux.
In both Linux and Windows, switching to fullscreen mode now shows 'odd' values for zoom. On my two systems, Linux shows 752% and Windows shows 401%. There is obviously some rounding errors here that could be accounting for the flashing. Those values should always be a multiple of 25%.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 2nd points was an easy fix. And the 3rd point is a direct result of the bezels. They have variable sized transparent windows (e.g. 1437 x 1077, enable the output in `Bezel::load()' to see values). So when we go fullscreen, the bezels and their window size define the maximal TIA zoom. Therefore this can be any value now.
I have no clue regarding the flashing, since it does not happen on my side with any renderer (Direct3D, OpenGL and software). Also not when in case of the now fixed bug of your 2nd point. Have you tried the software renderer?
Obviously I need your help here. But where to start?
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a start you could try to disable the
myBezel->render()call. And alternatively theTIASurfacerendering.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that when the call to
myBezel->render()is disabled, the TIA rendering is fine (in windowed and fullscreen modes). And the 'odd' zoom levels in fullscreen mode are irrelevant, and don't seem to be related. So now I need to dig into the Bezel code itself, to see why it's happening.Pretty obviously an interaction between rendering the two surfaces together, but what specifically I cannot say.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the driver doesn't like odd sizes. Due to rounding errors the bezel might become e.g. 1919 x 1079.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to mention that it's still happening in Windows fullscreen mode (but fixed in windowed mode). For Linux, it's present in both modes.
Both computers have AMD graphics cards. I don't have anything else to test on ATM. I'm off work next week, so I won't have access to different systems until Sept. 5 or so.
The bezel is rendering fine; no flashing or any other issues. It's the TIA image in the middle that flashes. Almost looks like a sync issue. Could also be related to the the blending used by different surfaces. In TV mode, the TIA is rendered solid, the scanlines are alpha-blended, and then the bezel is alpha-blended again.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another test case: changing the code to render the bezel before the TIA eliminates the flashing. Of course then you don't get the curved TV screen frame around the TIA image. But this points to the problem being related to how the two surfaces are being drawn.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. That would imply that the bezel's alpha is ignored every 2nd frame. Which seems very weird. Does the situation improve if you disable the scanlines?
BTW: I have an old NVidia (NVS310) and the AMD Vega build into my 5700G CPU. Testing on a 1280x1024 and a 1920x1080 monitor.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, was away from the computer for a while, pressure washing the deck.
It also happens in ROM mode when pressing 'Tab' to enter the menu. Mousing over a button and then off again causes the flicker to happen. So you can clearly see that one frame is blank, and the other is drawn. The last time this happened, it involved the logic in
FrameBuffer::update. I don't suppose you touched that, did you? I will check that out next.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very lightly refactored the code (merging TIA and bezel rendering into one method). I just double checked and I don't think I broke anything there.
Maybe I should reverse the argument order, though.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No luck finding the bug so far; will try to continue tomorrow.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am currently trying to allow rendering the bezel only once if it has no rounded borders. For that I disabled
clear()inrenderTIA(). This causes the bezel to flicker if I use theOpenGLordirect3d11renderer. Other renderer are doing fine.The flicker disappears when I
mySurface->setScalingInterpolation(ScalingInterpolation::sharp)with...::blurinBezel::apply()Obviously the double/tripple buffering is causing this. So, if the TIA window flickers, maybe it is not rendered every frame. Maybe some "optimization" of SDL2 and/or the renderer is causing this?
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear on the terminology, a call to
render()only sends the updated data to the underlying texture. The final call torenderToScreen()is what sends the updated textures to the output (ie, what we see). So there are two possibilities;render()is not being called at the right time, orrenderToScreen()is not being called all the time. And the current logic only callsrenderToScreen()when there have been actual changes to the framebuffer. This is definitely an optimization, so we don't have UI modes blitting at 60fps when nothing has changed onscreen.In TV mode, there are two things being rendered; the TIA image texture, and then the scanline texture. If we then pause, yet another texture is used to dim the screen. All of this worked fine before, so it's clear that multiple textures can work together. So I don't see how adding yet another texture (the bezel) can cause a problem. If it's sync'ed with the other ones, I see no reason why it doesn't 'just work'.
Note that the code you've added in your recent experimentation (to count frames to use for rendering) is something I tried before with the main rendering code. It lead to complicated logic that never worked in the end. We need to get all the 'renders' happening in one frame, and then call
renderToScreen()at the end. It really seems to be a sync issue.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot see how things have changed by adding the bezel rendering. But I will continue to debug the issue.
I know, it is more like a hack. I have an idea for a better solution already.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My latest commit might look like it fixes the problem for you when using non-rounded bezels (like the one for River Raid I pushed into
test/bezels). If that's the case, please check that the output isn't just skipping frames instead of blanking them.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following is for windowed mode:
Starting River Raid from the launcher, sometimes the bezel is flickering, sometimes not (on different runs). In either case, opening the options menu with 'Tab' leads to the TIA image alternating being rendered every time the mouse moves into and out of a button. IOW, every time a UI redraw is required, the underlying TIA image is only drawn half the time. This is also something I've seen before, which is why in UI mode involving the TIA, I force all underlying textures to be re-rendered. That doesn't seem to be happening here.
Also, I notice in your experimental code that you're calling
clear()twice, around a call torenderToScreen(). I've also tried that in the past, without much success. Some systems could have triple-buffering, in which case this idea won't work. @DirtyHairy actually pointed me to the current approach: we can't assume anything about the number of buffers; just act on the current frame. Render all things that need to be rendered, then push them to the screen. That works in single, double, and triple buffered modes. Any other approach just complicated the logic, and ultimately doesn't work on all platforms.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that's from my experiments (see below).
Agree, the question is why? And why would adding bezel render call change anything? If you remove that call, the problem goes away, right?
I only added this because of the remark regarding performance on old systems when using
clear(). Since it will not work reliably, I will remove it now.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, removing the bezel and the TIA/UI renders correctly. And if I swap the order of rendering in
renderTIA, and have the bezel drawn first and then the TIA image, the flickering disappears (except for the changes from your experiments). I have absolutely no idea why this occurs. The only difference is that the bezel is using the alpha-channel in a way we've never done before.adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which SDL version are you using? Have you tried other ones?
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2.26.5, the latest available for Linux. I can't use anything newer than that, since 2.28 has compile issues (in SDL itself) on my distro.
EDIT: However, I'm using 2.28.0 on my Windows machine, and there the TIA/UI is still flickering when in bezel mode.
EDIT2: Also tried another Linux install on my Windows machine (dual boot). That one is using a different version of Linux, a different version of SDL, and also a different rendering backend (Wayland vs. X11). Very similar results to the other testing.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try an older version, just in case? Though, I am using 2.65.5 too.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will take a little while, since I will have to compile it and make Stella link to it, vs the system-wide version.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I thought it would be easy. Probably not worth the effort then.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, just realized I had a CHROOT of Ubuntu 20.04 with SDL 2.0.10. Tried that, and still getting the same results.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should bring this to Gitter/Element. Maybe @DirtyHairy has an idea.
adc7a3cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, go for it.