RFC: Fullscreen rendering with configurable aspect ratio #1080
Conversation
|
I don't know why you called configurable aspect ratio since you don't do that, at least I don't see it in the code, I would call it fullscreen scaler modes. I like "modes" idea instead resolutions, however not all from it. Please explore rendering using this extension which might be solution for us. Rendering to texture in OpenGLS I guess is fine. About whole framework I know it's just make some solution for fullscreen issues, but you should know that is not direction I would see here. Might be solution for time being to get proper framework. And TinyGL also need support this. This PR is about fullscreen, but what about window mode ? On another topic, but related to this changes, I don't like idea moving more and more OpenGL/GLS code there, why not use similar approach with code in "graphics/opengles2" and make "graphics/opengl" for helper functions. Also move some existed parts from SDL surface file if possible to that dirs. also probably from android backend too. |
|
IMHO, TinyGL internally doesn't need to support scaling (internally), as long as we allow for some scaling (where possible) of the final frame buffer atleast (which is similar to rendering to an FBO anyhow). |
| return 1; | ||
| --k; | ||
|
|
||
| for (uint i = 1; i < sizeof(T) * 8; i <<= 1) |
There was a problem hiding this comment.
IMHO, loops should always have braces.
|
Can we render at native or arbitrary resolution? IIRC it was possible manually changing variables currently set to 640 and 480. |
|
@FabioPedretti: @aquadran: |
| @@ -40,26 +53,53 @@ static bool usePackedBuffer() { | |||
| return true; | |||
| } | |||
|
|
|||
| FrameBuffer::FrameBuffer(GLuint texture_name, uint width, uint height, uint texture_width, uint texture_height) : _colorTexture(texture_name), _width(width), _height(height) { | |||
| FrameBuffer::FrameBuffer(uint width, uint height) | |||
| : _managedTexture(true), _width(width), _height(height), | |||
There was a problem hiding this comment.
formatting: initializers should not be at same level as code
|
"For windowed mode you either need a target resolution or a scaling factor, which could be the input to the frame buffer" this statement is in conflict with this PR where the same thing can work too. This could be: Still not sure about your case "native", it doesn't make much sense for HW accelarated scaling, but in case software, turn off for some might be an option because of speed, like speed issues in AmigaOS4 port. I see this PR only with included window mode too. |
|
Excellent idea. I would keep it as is, with:
Regarding the current implementation, I am experiencing the following issues (mesa/r600g):
APITrace does not show any error. |
|
@bgK option for filtering might be indeed needed even for TinyGL case. |
|
Doesn't this, for the fullscreen case atleast, boil down to:
For windowed mode, I can see having other settings (like integer multiple), but for fullscreen? |
|
different options for fullscreen and for window make mess in config and handling switching fullscreen<->window mode. That require two sets of configs, two sets of defaults, two ways GUI options handle. Unified options which works for both make more sense for me. I'm pointing cases when for us something make not much sense, but for users it is. Like stretch mode which some prefer. All options can work for both if we allow it. |
|
Yes, it boils down to how to layout this in configuration. Stretch is basically a question of "maintain aspect ratio", which could easily be a checkbox setting (and thus make sense both for windowed and fullscreen with the same setting). The bigger question is how we would allow users to select scale factor for windowed, while having a sensible possibility of "fill the screen (possibly with pillboxes)" for fullscreen. Since the resolution necessary to do so depends on the desktop resolution (or our ability to change it in the case of fullscreen), and that scaling anywhere between 640x480 and filling the screen is a use case that is useless for fullscreen, I'd suggest this:
In fullscreen, if scaling is enabled, then the scale factor should be ignored, and filling the screen is the behavior that should happen (possibly stretching). If maintain aspect is also enabled, then we should only scale by the same factor in both directions, and the factor should be the one that (as bgk says) fills whichever comes first of height/width, but no more. If however we are in windowed mode, then the user selected scale factor takes precedence. Another alternative, would be to expose actual resolutions to the user, as well as the maintain aspect setting. (Which we will want anyhow, to allow the users to override any problematic screen setups, so that they ALWAYS have the possibility of getting pillarboxes, as that aspect is how the game was designed to be played). |
Botje
changed the title
Raise an assertion if a framebuffer is requested without the correct extension
|
These patches make it work on OpenGL as well, and make scaling take up all of the smallest dimension, leading to pillarboxing. |
|
I think this is the behavior @somaen and @bgK suggested. This does leave the comments about |
|
Regarding OSystem::{getWidth,getHeight}, the actual resolution makes no sense from the perspective of an engine. Therefore IMO we should return the game screen resolution. I've pushed some changes building upon this PR to this branch:
@Botje, you may or may not want to include some of these commits here. Together @Botje's changes, merging would be fine with me once falling back to mode setting is implemented. |
| @@ -260,17 +296,17 @@ Graphics::PixelBuffer SurfaceSdlGraphicsManager::setupScreen(uint screenW, uint | |||
| SDL_GL_GetAttribute(SDL_GL_STENCIL_SIZE, &glflag); | |||
| debug("INFO: OpenGL Stencil buffer bits: %d", glflag); | |||
|
|
|||
| #ifdef USE_OPENGL_SHADERS | |||
| debug("INFO: GLSL version: %s", glGetString(GL_SHADING_LANGUAGE_VERSION)); | |||
|
|
|||
There was a problem hiding this comment.
why you moved out USE_OPENGL_SHADERS out of include GLEW code ?
|
If there are no more comments I'll do a final round of testing with all variations I can think of, push my |
|
this is not full functional implementation, it lacks of tinygl support |
|
is aspect ratio button disabled for non fullscreen mode ? |
I think that can be done in a separate PR, the OpenGL implementation can be introduced without breaking TinyGL, as long as TinyGL is still doing something sane and not crashing or whatever. |
|
I don't agree. if this will not be implemented this stay open at it is, until missing feature is implemented on separated PR |
|
Why? Are you willing to implement it? |
|
"Why? Are you willing to implement it?" please don't start with that, I didn't started this PR. This PR not describe only OpenGL/ES2. I'm not accepting this as only because "don't know" or "willing not do that". This is not impossible or hard. It's not even showing try implementing. |
|
You are throwing away good code and a working feature. The "Are you willing to implement it?" was an honest question. Botje has all the rights to not wanting to write that, on the other hand this PR works. If you keep this line this feature may never come to ResidualVM, for any renderer. |
|
I'm not throwing any code but holding it until this feature will be fully workable. I'm not telling him write any code. But I can block any code if this doesn't fit proper quality/functionality. I prefer something finished not half done imported into main code. |
|
What makes you think there will ever be a TinyGL implementation? It may even be impossible or require changes too massive to be worth it. |
|
"What makes you think there will ever be a TinyGL implementation? It may even be impossible or require changes too massive to be worth it." Why so negative statements? "ever", "not possible", "not worth it". |
|
Look, I agree with striving for feature parity when it comes to engines and renderers. |
|
@Botje So, in that kind of thinking why keep TinyGL because small range of people use it? We could also remove classic OpenGL renderer, why we bother use version below 3.0. we could also drop SDL1 for SDL2, Why we bother about amigos4 users? Due percentage we could also drop OS X and linux, since mostly are Windows users. in that kind of thinking why RE EMI, since it works on windows. |
|
I'm not advocating the removal of TinyGL. I just don't see the point in holding back something that people on the forum have asked about several times because it does not improve the experience for a completely hypothetic situation. |
|
To be completely fair, full feature parity with regards to scaling will NEVER happen at all, for TinyGL the only thing that might make sense, without slowing everything down quite a bit, would perhaps be a nearest neighbour 2x/3x-solution. Overall, I personally think that allowing scaling in TinyGL is a bit separate from allowing it in OpenGL, especially if we have any kind of requirement of speed on it. (For instance, accelerated scaled blitting on platforms that allow it, is ENTIRELY out of scope for this discussion). At some points, TinyGL simply can't keep up successfully with OpenGL, which is quite clear, HOWEVER, in this case, I'd say this: The major issue is that we have:
The solution to this, might be to simply have a different mode where the TinyGL buffer is scaled by SDL, allowing for it to fill the screen. We might even be able to speed this up a tad by leveraging the Dirty-rect-rect when talking to SDL, so that SDL doesn't need to do a full blit every time it updates. So, if we're not doing any sort of scaling, same thing as now, if TinyGL needs to be scaled, we handle it SDL-side? Other than that, I can see the that TinyGL-scaling could be handled as a separate issue from this PR in itself, as the code doesn't touch at all. ScummVM has different degrees of scaling available depending on renderer too (OpenGL allows arbitrary window-sizes, the various software-scalers allow only 2/3/4(?)x, where some only allow 2x). |
|
"people on the forum have asked about several times " I don't want answer for that here on public channel. I have always simple answer for that. |
|
@somaen in my case OpenGL drivers are broken, lighting is wrong, so for proper draw I need use software renderer, fortunately OS make aspect ratio on system in fullscreen. |
|
Well, the speed vs CPU-developments question is a bit problematic, most of the platforms where OpenGL isn't working, are also platforms where the CPUs are in the slower end of the scale. For platforms with faster CPUs, there is likely to be decent OpenGL-support too (and thus more relevant to fix the issues with the drivers in OpenGL). Overall, this boils down to whether anyone wants to fix the TinyGL-case to allow for consistent scaling. (Speed is atleast completely out of scope for THIS discussion). So, if this is merged without TinyGL changes, then we have a system where we get:
I can understand that that is a bit partial, but consider the current scenario, where ALL renderers end up doing what TinyGL does now (leaving it up to the OS). Thus, this PR is an improvement, without moving TinyGL backwards in any other way than adding an option it ignores. I suggest the following compromise: We merge this, thus allowing for it to be fully tested before any release. Then we can perhaps discuss what is enabled in the release, depending on whether we can make TinyGL decently consistent with the options. |
|
There. It compiles and runs. |
|
I just compiled and tested with EMI: OpenGL: alt-enter switches mode, and does fill the screen, but switching back to window, I get a buggy window with the font data displayed all over the screen, instead of the game window (it's also problematic that the enter-call gets passed through to the game). This might stem from issues with how the engine SWITCHES between fullscreen and windowed. TinyGL: Switching to fullscreen, I get a tiny window, then nothing ever gets updated when I switch back. |
|
@somaen "most of the platforms where OpenGL isn't working, are also platforms where the CPUs are in the slower end of the scale" this is wrong statement, GPU and CPU are not correlated. you can have decent platform with strong CPU but broken or lack proper driver. |
|
I have no idea which path the code takes when you hit alt-enter or why it's behaving so weird. |
|
That should fix switching between windowed and full screen. |
This is my proposal to enable ResidualVM to work on monitors/drivers/OSes that no longer allow switching to 640x480.
In the shader-based renderer (and only there, for now), the game is first rendered to a framebuffer and later pushed to the screen.
You can configure how this is done using the
fullscreen_modeparameter per-game.There are four options.
Assuming your desktop resolution is 1920x1080, and the game wants a 640x480 window, these are:
native: This is the current way of rendering, 640x480 resolution, no framebuffer.center: The game is rendered as a 640x480 rectangle in the center of your 1920x1080 display.scale: The game is rendered to 640x480 and scaled up to an integer multiple that still fits. For the given resolutions, a 1280x960 rectangle in the center of your 1920x1080 display.stretch: The game is rendered at 640x480 and stretched to fill your 1920x1080 display.If you think this approach could work, I'll look into expanding it to use GL_EXT_framebuffer_object for OpenGL 1.4.