Allow setting window's default size

[haanhvu]

Fix #1491

Allow users to choose window's default size in Display settings. There are four options: 800x450 (default), 750x500, 825x550, 900x600.

For 900x600, the Window Resize feature that is existing in each window will only allow 1x at maximum, because bigger scales don't work.

[haanhvu]

@felipeerias Currently the radio buttons for setting windows' size have appeared. But for some reasons they don't have effects on setting windows' size yet. It seems like there're still some logics missing to actually size the windows?

Update: I just found some mistakes in my current code. Please wait until the next commit (that will fix those mistakes) to review my changes.

Update: Fixed but the behavior doesn't change yet. Seems like we need to update some other places (WindowWidget?) for the changes in window's size too? 🤔

[haanhvu]

Update: Fixed but the behavior doesn't change yet. Seems like we need to update some other places (WindowWidget?) for the changes in window's size too? 🤔

@felipeerias After further investigation, I found out that in many places we are using WINDOW_WIDTH/HEIGHT_DEFAULT directly, instead of calling getWindowWidth/Height(). These are probably where we need to update to make window sizing actually work. Let me try

[haanhvu]

@felipeerias As discussed in chat, this feature (default window sizing) works now, but the existing per-window Window Resize feature doesn't work or works incorrectly for some sizes.

Please help me figure out when you have time. Maybe we need to revise the current logic of the Window Resize feature. I'll take a look at that.

[svillar]

Instead of adding the WIP preffix you can just set this as a draft. Reviewers would understand that it's WIP and it won't appear as ready to be reviewed

[haanhvu]

@svillar @felipeerias This is ready for review now. Besides allow setting window's default size, I also restrict Window Resize feature's max scale option for 1000x750 size to be 1x. Because higher scales didn't work as I checked.

Video: https://drive.google.com/file/d/1Dx-7Kl_WN3jCDPjydHEcPPv4GA39WPEF/view?usp=sharing

[felipeerias]

This is just an idea, but maybe we could put the size presets in an enum in SettingsStore.

public enum WindowSizePreset {
    PRESET_0(800, 450),
    PRESET_1(800, 600),
    PRESET_2(1000, 562),
    PRESET_3(1000, 750);

    public final int width;
    public final int height;

    WindowSizePreset(int width, int height) {
        this.width = width;
        this.height = height;
    }
}

public final static WindowSizePreset WINDOW_SIZE_DEFAULT = WindowSizePreset.PRESET_0;

For the descriptions, we could use just one string:

<string name="window_size_preset">%1$d×%2$d</string>

getContext().getString(R.string.window_size_preset, preset.width, preset.height);

We would need to populate the radio button group programmatically by iterating that enum:

List<String> windowSizePresets = new ArrayList<>();
for (SettingsStore.WindowSizePreset preset : SettingsStore.WindowSizePreset.values()) {
    windowSizePresets.add(getContext().getString(R.string.window_size_preset, preset.width, preset.height));
}
mBinding.windowsSize.setOptions(windowSizePresets.toArray(new String[0]));
mBinding.windowsSize.setOnCheckedChangeListener(mWindowsSizeChangeListener);

When the user selects a radio button, I think that the id that we receive is just its position in the enum, so we could use it like this:

private void setWindowsSize(int checkedId, boolean doApply) {
    ...
    SettingsStore.WindowSizePreset sizePreset = SettingsStore.WindowSizePreset.values()[checkedId];
    // store this value in the settings
    ...

I'm not sure yet how we would store this in the Settings. Maybe we could store an integer to reference the selected enum value, but I don't know if there might be a better solution.

[felipeerias]

This seems fragile. Could we find a better way to implement it?

[haanhvu]

@felipeerias I agree the last approach seemed fragile. But I think using enum in this case is a bit overkill. I tried a simpler approach with String[] WINDOW_SIZES = {"800x450", "800x600", "1000x562", "1000x750"} instead. I tested and it worked on my Quest 3. And the code also looks simpler now.

Please review whether this new approach makes sense. Thanks!

[felipeerias]

@haanhvu It is less code but we are still storing integer values in strings, which seems unnecessary and harder to maintain. We might want to add more presets later on, or change how they are calculated. Keeping the data in its natural format makes it more flexible to handle.

[haanhvu]

@felipeerias I applied the new enum type in the latest change. Pls review again, thanks!

[felipeerias]

@haanhvu This PR look good to me. Good job!

Before we integrate the changes, could you merge them into one commit.

I have been testing with different default sizes and the largest size that seemed comfortable was 900x600, so maybe we could take that as a reference and change the presets like this:

PRESET_0(800, 450),
PRESET_1(750, 500),
PRESET_2(825, 550),
PRESET_3(900, 600);

[haanhvu]

@haanhvu This PR look good to me. Good job!

Before we integrate the changes, could you merge them into one commit.

I have been testing with different default sizes and the largest size that seemed comfortable was 900x600, so maybe we could take that as a reference and change the presets like this:

PRESET_0(800, 450),
PRESET_1(750, 500),
PRESET_2(825, 550),
PRESET_3(900, 600);

Done!

[felipeerias]

Thank you for the changes, this is almost ready to merge.

[felipeerias]

Looks good to me. It works very well. Good job!

[felipeerias]

A few minor issues.

[haanhvu]

@felipeerias I have resolved all your reviews. Besides that I have also set the RadioGroup to be in vertical orientation if there are from 4 radio buttons, like this:

Because if 4 radio buttons are in horizontal orientation they would easily overlap with the radio description, especially when the description is long like this case.

Please review again, thanks!

[haanhvu]

@felipeerias I have applied the new solution for overlapping buttons and fixed the string resource like we discussed in chat.

I also increased the space between the description and the buttons a bit as we discussed in chat. This is how it looks: