rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
117 stars 17 forks source link

First attempt at splitting top screen types #76

Closed ian-h-chamberlain closed 1 year ago

ian-h-chamberlain commented 1 year ago

Closes #69

I'm not totally happy with this API, it feels like there's a bit of repetition but I think this is generally the gist of what I was thinking. I haven't updated all the examples yet since I wanted to get some opinions first.

I don't love having the extra inner type, but just using a RefCell<TopScreen> would force callers to use Ref{Mut}::map which is kind of a pain and also requires exposing the left and right members, so idk.

One thing that's kinda missing here is a type-safe way of working with a wide mode screen, for example it might be nice to have something like this:

struct TopWideScreen;
impl Screen for TopWideScreen {
    // use left-side frame buffer
}

// maybe no impl Screen for TopScreen, then?

impl TopScreen {
    pub fn wide_mode(self) -> TopWideScreen;
}

impl TopWideScreen {
    pub fn unset_wide_mode(self) -> TopScreen; // or some other way to disable wide mode
}
Meziu commented 1 year ago

This is a big issue, especially when keeping in mind citro-3d. The idea of splitting the types sounds fine, but we must check for all safety measures. For example: When enabling wide-mode, both TopScreen sides should be available. Also, does rendering to the right side work when not in 3d-mode? Can we actually use all of the screens (left and right side counted as different) independently?

AzureMarker commented 1 year ago

Is it legal/safe to enable/disable wide mode while already using the top screen instances? I think it works, so not sure if we should block it.

Meziu commented 1 year ago

Is it legal/safe to enable/disable wide mode while already using the top screen instances? I think it works, so not sure if we should block it.

I believe the RightSide would become invalid, or just do nothing. I haven’t tested it.

Meziu commented 1 year ago

After looking at libctru's code, I can firmly declare that: the Right side is a lie. There is no such thing as a Right side. The framebuffer size is always 2x the TopScreen pixelcount (depending on the format). When getting the RightSide's framebuffer, it only returns the LeftSide's framebuffer's (which we might as well call the "TopScreen framebuffer") pointer with an offset. The additional RAM is allocated even if the program never needs 3D or WIDE mode.

In my opinion, it's a bit wacky, but it also means that the RightSide is valid as long as the gfx service is running. Furthermore, the RightSide isn't flushed to the screen unless 3D mode is activated (or its memory is used by wide mode): it cannot be used as a standalone screen.

At this point, I believe the best approach would be to remove the "get framebuffer based on side" thing and just split the types when asked by 3D mode activation. This way we can make safety checks for the use of wide-mode (or even just the left side alone) directly, and not worry too much about it.

Something like:

impl TopScreen {
        // We get the 2 sides when needed for 3D
    pub fn enable_3d_mode(&mut self) -> (TopLeft, TopRight);

    // The 2 sides get dropped inside this function to assure the correct deactivation of 3D mode
    pub fn disable_3d_mode(&mut self, TopLeft: Side, TopRight: Side);
}

@ian-h-chamberlain @AzureMarker What Side (as in the trait/type) should be in this case is up to debate. It shouldn't be a "full" version of the screen, since the sides can't independently change framebuffer format or set double buffering, which would be instead handled by the already existing TopScreen instance. Tell me your ideas to implement this.

AzureMarker commented 1 year ago

Those functions would need to take ownership of the TopScreen value, otherwise you could make multiple left and right values. It could return a third value representing the top screen in general.

ian-h-chamberlain commented 1 year ago

Hey all, I've just pushed a change based on the discussion, which I think mostly accomplishes the goals of having a distinct type to split the top screen. It's not quite what I first envisioned, but keeps the RefCell somewhat like it was before.

I also added an example to make sure it works as expected when writing to the left/right frame buffers and it seems like it works well! Proper Citro3D stereoscopic might work a bit differently, but I think could end up taking ownership of a RefMut<dyn Screen> to prevent writing directly to the framebuffer while C3D is rendering to the screen.

ian-h-chamberlain commented 1 year ago

@rust3ds/active anyone have objections to merging this, or other comments?