FF7: Added experimental 16:10 support

[dotaxis]

Summary

Crops 16:9 aspect ratio horizontally to support 16:10. Adds a new entry to the FFNx.toml to differentiate between 16:9 and 16:10.

There may be a much better way to do this, but I wanted to at least have a discussion about it. Mod authors will need to add support for this if it reaches production.

Motivation

The Steam Deck uses a resolution of 1280x800, and this will remove the black bars on the top and bottom of the screen.

ACKs

• I have updated the Changelog.md file
• I did test my code on FF7
• I did test my code on FF8

[julianxhokaxhiu]

Thanks for this PR!

@CosmosXIII @tangtang95 can you please review?

[tangtang95]

It seems a good starting point. But please move the override of wide variables to the Widescreen init function and then move the call of widescreen.init() before newRenderer.init(). I think it might be better to have this logic into Widescreen class even though the renderer depends on these variables

Then there are also other global variables that could be overwritten in the widescreen.cpp file.

Overall I think it is ok for now

[dotaxis]

If you are referring to these globals:

int viewport_width_plus_x_widescreen_fix = 750;
int swirl_framebuffer_offset_x_widescreen_fix = 106;

I don't really understand how these values are calculated. I thought that the first one might be wide_viewport_width + wide_viewport_x + 3 and the second would be -wide_viewport_x - 1 but I didn't want to just guess and give the impression that the new values were proper. Can you give some insight on that?

I will make the other changes you mentioned in the meantime.

[tangtang95]

More or less, that is how you calculate it. But I don't remember where those fixed numbers came from. Either I tweaked it to fix something or I tweaked the wide variables in widescreen.h and forgot to updates these ones.

For now just ignore the fixed numbers

[tangtang95]

@dotaxis I think that here https://github.com/julianxhokaxhiu/FFNx/blob/master/src/gl/special_case.cpp#L74C58-L74C58 we need to use a new variable that is the aspect ratio in float. By default it is 16/9.f, but in your case, it should be 16/10.f. Otherwise, external texture images for buster_tex (background image of new game/continue menu) will be using a 16/9 image squashed into 16/10.

[dotaxis]

Right. I did have something there in my initial draft but I forgot to fix it when I got ready for a PR. Will fix.

[tangtang95]

Ok, for me the PR is good now

[julianxhokaxhiu]

I was looking more carefully at the PR and I'd like not to introduce another flag for controlling Widescreen. We already have aspect_ratio which is enough.

Therefore I propose the following changes:

• Remove widescreen_ar ( flag, toml section, etc. )
• Add a new option 3 for aspect_ratio
• Update the FFNx.toml to something like this:

#[ASPECT RATIO]
# Preserve original game aspect ratio of (4:3) by adding black bars on the left and right side (if needed)
# When off the game will be stretched to fit the window's aspect ratio; Be aware the game may look wrong though.
# 0: Preserves original game aspect ratio of (4:3) by adding black bars on the left and right side (if needed)
# 1: Stretched to fit the window's aspect ratio
# 2: 16:9 aspect ratio mode without stretching the image
# 3: 16:10 aspect ratio mode without stretching the image
#~~~~~~~~~~~~~~~~~~~~~~~~~~~
aspect_ratio = 0

• Update the constant AR_WIDESCREEN to AR_WIDESCREEN_16x9
• Add a new constant AR_WIDESCREEN_16x10
• Use this constant as a discriminator in the new if condition where 16:10 values are set

This will keep the code cleaner, scalable and more robust, as well as simplify the addition of this option on 7th later.

Thank you in advance

[tangtang95]

I was looking more carefully at the PR and I'd like not to introduce another flag for controlling Widescreen. We already have aspect_ratio which is enough.

Therefore I propose the following changes:

• Remove widescreen_ar ( flag, toml section, etc. )
• Add a new option 3 for aspect_ratio
• Update the FFNx.toml to something like this:

#[ASPECT RATIO]
# Preserve original game aspect ratio of (4:3) by adding black bars on the left and right side (if needed)
# When off the game will be stretched to fit the window's aspect ratio; Be aware the game may look wrong though.
# 0: Preserves original game aspect ratio of (4:3) by adding black bars on the left and right side (if needed)
# 1: Stretched to fit the window's aspect ratio
# 2: 16:9 aspect ratio mode without stretching the image
# 3: 16:10 aspect ratio mode without stretching the image
#~~~~~~~~~~~~~~~~~~~~~~~~~~~
aspect_ratio = 0

• Update the constant AR_WIDESCREEN to AR_WIDESCREEN_16x9
• Add a new constant AR_WIDESCREEN_16x10
• Use this constant as a discriminator in the new if condition where 16:10 values are set

This will keep the code cleaner, scalable and more robust, as well as simplify the addition of this option on 7th later.

Thank you in advance

I think this was the previous solution proposed in Discord, but this required to change also all the if statement where aspect_ratio == AR_WIDESCREEN was used. I suggested to create a new flag in order to avoid modifying all those if statements. It wasn't really scalable in case another aspect ratio will be implemented (e.g 21:9, but this won't happen as it would be incompatible)

[dotaxis]

I think this was the previous solution proposed in Discord, but this required to change also all the if statement where aspect_ratio == AR_WIDESCREEN was used. I suggested to create a new flag in order to avoid modifying all those if statements. It wasn't really scalable in case another aspect ratio will be implemented (e.g 21:9, but this won't happen as it would be incompatible)

Yeah, I think the current solution is much cleaner than having a fourth aspect ratio because of the way FFNx handles 16:9 (many different checks to the value of aspect_ratio throughout the codebase).

However, it may work to use a bool widescreen_enabled = (aspect_ratio >= AR_WIDESCREEN_16X9 && aspect_ratio < AR_COUNT) and replacing all the aspect_ratio checks with if (widescreen_enabled) or similar.

That said, I think this is not really scalable either way since 16:10 is pretty much the only other widescreen resolution that we can reasonably support.

I can try out the changes suggested by @julianxhokaxhiu, but I'm not sure if the end result is actually going to be cleaner than this. I'll try.

[julianxhokaxhiu]

I think this was the previous solution proposed in Discord, but this required to change also all the if statement where aspect_ratio == AR_WIDESCREEN was used. I suggested to create a new flag in order to avoid modifying all those if statements. It wasn't really scalable in case another aspect ratio will be implemented (e.g 21:9, but this won't happen as it would be incompatible)

Yeah I know that the PR may seem "heavier" on the diff side of things, but it will feel much more lightweight as if we'll need to cover more aspect ratios in the future ( who knew we would even be discussing native 16:10 for example ) we can simply use one flag to rely on that info instead of two, which one depends on the other. I'd prefer to streamline the approach even if we have to "bulk rename" a constant everywhere basically.

I can try out the changes suggested by @julianxhokaxhiu, but I'm not sure if the end result is actually going to be cleaner than this. I'll try.

Thanks a lot, appreciated

[dotaxis]

I have come up with what I believe is an acceptable, scalable solution. Please review.

The only caveat here is that mods that read aspect_ratio from the toml will think the game is running at 4:3 until they are updated. This may actually be better than the previous solution because now UI elements aren't getting cut off the edges of the screen.

[julianxhokaxhiu]

Allright, the PR LGTM :) Thanks @dotaxis!

@tangtang95 @CosmosXIII if you can have a final look I'd appreciate. By looking at the code I can't notice any particular drawback, on the opposite site the code can scale accommodating even new ARs eventually. Any drawbacks I can't see?

[tangtang95]

Yeah, seems good to me. In this way, it will be simpler for user and also if we want to extends 7th graphics driver config

[tangtang95]

I have come up with what I believe is an acceptable, scalable solution. Please review.

The only caveat here is that mods that read aspect_ratio from the toml will think the game is running at 4:3 until they are updated. This may actually be better than the previous solution because now UI elements aren't getting cut off the edges of the screen.

It's better like this, so mods will have to be updated in order to support 16:10.
Anyway, nice work overall! 🎉

[dotaxis]

@julianxhokaxhiu shouldn't this be a bool?

FFNx/src/globals.h

Line 69 in e7aef84

extern uint32_t widescreen_enabled;

FFNx/src/common.cpp

Line 121 in e7aef84

uint32_t widescreen_enabled = false;

Edit: Maybe not, but I'm wondering what the reason for the type change is.

[julianxhokaxhiu]

Edit: Maybe not, but I'm wondering what the reason for the type change is.

The bool type is anyway an integer in memory, and it uses 4 bytes. The current type will allow us to scale this flag eventually to a third or forth state in the future if needed :)

Yeah, seems good to me. In this way, it will be simpler for user and also if we want to extends 7th graphics driver config

Thanks a lot to you for reviewing!

[dotaxis]

Edit: Maybe not, but I'm wondering what the reason for the type change is.

The bool type is anyway an integer in memory, and it uses 4 bytes. The current type will allow us to scale this flag eventually to a third or forth state in the future if needed :)

That makes sense. Thanks for explaining.

And thank you both for helping me get this up to spec!

[julianxhokaxhiu]

Thanks a lot @dotaxis great PR! Looking forward to next ones :)