Flawless Frames feature

[grondag]

Addresses vram-guild/canvas#282

Pasting relevant javadoc here for convenience.

/**
 * FREX renderer access for an entry point mods like ReplayMod can use to signal the
 * renderer that rendering quality should be favored over speed because output is being
 * automatically recorded. If a renderer is playing loose with some visual elements
 * to achieve interactive frame rates it should try to disable those measures if it will
 * give a noticeable boost to quality.
 *
 * <p>At a minimum, the renderer should ensure that terrain iteration blocks the main
 * render thread until the entire visible set of chunk sections are determined and all
 * visible chunk sections have been built and can be rendered in the frame.
 *
 * <p>While this API is defined as part of FREX, it uses standard Java interfaces so that
 * it can be implemented and consumed without reference to any FREX library or any other
 * mod dependency. This is done to facilitate adoption by renderers that do not implement
 * FREX and to spare mods that do not use other FREX features from including it.
 *
 * <p>This interface and it's implementation are provided as a convenience for FREX renderers.
 * Non-FREX renderers can copy the relevant code from FREX or create their own implementation as desired.
 *
 * <p>Mods that want to request control of this feature should implement the {@code frex_flawless_frames}
 * entry point, as illustrated below. The entry point class must implement
 * {@code Consumer<Function<String, Consumer<Boolean>>>}.
 *
 * <p><pre>
 * "entrypoints": {
 *    "frex_flawless_frames": [ "yourorg.yourmod.yourpackage.MyConsumer" ]
 * }</pre>
 *
 * <p>Renderer implementations should ensure every mod that declares this end point receives exactly
 * one call to provided consumers. (For renderers using FREX, the provided implementation satisfies
 * this guarantee.)
 *
 * <p>When an end point is invoked, the mod consumer will receive a {@code Function<String, Consumer<Boolean>>}
 * instance.  The string name passed in to this function is meant to facilitate logging and debugging
 * of activation within the renderer when multiple mods may be using the end point. The resulting
 * consumer can then be used at any time to activate or deactivate this feature.
 *
 * <p>The contract for usage of the activation consumer is as follows:
 * <ul><li>Calls are thread-safe but not reentrant. (Only one thread should call a given
 * consumer, but it can be any thread.)
 * <li>Calls are idempotent - setting true or false multiple times in succession has the same
 * outcome as calling once.
 * <li>The renderer will check the current status at the start of a frame and that status
 * will be effective for the whole frame.
 * <li>Because of the above conditions, consumers needing precise control of frames should
 * change status on the render thread before the start of the next frame.</ul>
 *
 * <p>The feature will be active when one or more consumers have requested it and inactive
 * when no consumers have activated it.  FREX renderers can use {@link #isActive()} to query
 * the currently effective state.
 */

[grondag]

@jellysquid3 @Johni0702 please have a look

[Johni0702]

The renderer will check the current status at the start of a frame and that status will be effective for the whole frame.

I'm guessing your intention with this one was to put a restriction on the consumer, requiring that everything is configured before the frame starts, and allowing the renderer to freely cache that state for one frame as it sees fit. But currently it sounds like the renderer must cache the status at the start of the frame and then must not react to any changes until the next frame.
Can we relax this? E.g.

The renderer may check the current status at any time (in particular also at the very start of the frame) and is not required to check again until the start of the next frame.

Otherwise, LGTM 👍

[grondag]

I'm guessing your intention with this one was to put a restriction on the consumer, requiring that everything is configured before the frame starts, and allowing the renderer to freely cache that state for one frame as it sees fit. But currently it sounds like the renderer must cache the status at the start of the frame and then must not react to any changes until the next frame.
Can we relax this? E.g.

The renderer may check the current status at any time (in particular also at the very start of the frame) and is not required to check again until the start of the next frame.

We can relax it. Your interpretation is correct - I assumed no renderer would want to deal with mid-frame changes but there's no need to dissallow it.

Do you have a specific use case in mind or is this only for clarification?

[Johni0702]

Only for clarification. For Sodium and Bobby it'll just be simpler to check once at the place where the fast and the fancy code paths diverge (with both of them there is afaik currently only one such place), rather than having to do it specifically at the start of the frame.

[juliand665]

How would I best go about hooking into this with Dynamic FPS (to not reduce FPS while flawless frames are active)? You have a section on "Mods that want to request control of this feature", but I just want to check if it's enabled.

[grondag]

How would I best go about hooking into this with Dynamic FPS (to not reduce FPS while flawless frames are active)? You have a section on "Mods that want to request control of this feature", but I just want to check if it's enabled.

Interesting question. The current design doesn't anticipate that other mods would want to listen to the activation state, only that the renderer would. But I see how that could be useful.

If FREX is present, then it is easy: you can query FlawlessFrames.isActive(). But this is API is designed not to require FREX, it's possible Sodium or another renderer might not use that.

I suppose we could add another endpoint/interface, or maybe we spilt to this a separate (very tiny) library just so there's no reason not to use it?

[Johni0702]

Oh, I didn't even notice that the design doesn't intent for other mods to listen to the activation state. In fact, I was going to do just that with Bobby as well. It overall works perfectly well with multiple "renderer" present at the same time if we just drop the section requiring (well, technically it's only a "should" not a "must") that endpoints be only called once:

Renderer implementations should ensure every mod that declares this end point receives exactly
one call to provided consumers. (For renderers using FREX, the provided implementation satisfies
this guarantee.)

In fact, I wasn't even fully aware that this restriction existed (I must have read it more like "receives exactly one call per renderer implementation"), so, the endpoint I implemented in the ReplayMod already assumes that it may be called by multiple renderer and will simply call them all when it wants to activate the feature.

[grondag]

Oh, I didn't even notice that the design doesn't intent for other mods to listen to the activation state. In fact, I was going to do just that with Bobby as well. It overall works perfectly well with multiple "renderer" present at the same time if we just drop the section requiring (well, technically it's only a "should" not a "must") that endpoints be only called once:

Renderer implementations should ensure every mod that declares this end point receives exactly
one call to provided consumers. (For renderers using FREX, the provided implementation satisfies
this guarantee.)

In fact, I wasn't even fully aware that this restriction existed (I must have read it more like "receives exactly one call per renderer implementation"), so, the endpoint I implemented in the ReplayMod already assumes that it may be called by multiple renderer and will simply call them all when it wants to activate the feature.

The activation endpoint that's currently exposed is only for changing the status of the feature - turning it on or off. There's nothing for mods to listen to it.

The way Fabric Rendering API works, there can only ever be one active renderer and it is responsible for implementing the endpoint so it will naturally have access to the status (or it can use the FREX implementation).

That's why this spec didn't include a generic way for other mods to listen to the status. It's a non-issue if a mod depends on FREX but we already said for this feature we wanted to support renderers and mods that can't or don't want to depend on FREX directly.

There's an easy fix: just add a separate endpoint for mods to listen to activation status instead of controlling activation status. I'll add that.

[Johni0702]

There's an easy fix: just add a separate endpoint for mods to listen to activation status instead of controlling activation status. I'll add that.

But that will then require a renderer to be present and do the message passing between them when that should not really be required. Neither ReplayMod nor Dynamic FPS need one but they still want to communicate.

My point was that the current spec is not only easy to use as a mod wanting to change the status but also as one wanting to let other mods change your status by just acting as a renderer (in the narrow sense of this spec where a renderer is just some thing that influences how stuff gets rendered. not in the Fabric Rendering API sense where there can only be one).
You just ask FabricLoader for all frex_flawless_frames entrypoints, call each of them once with your setter and then they will call you whenever they wish to change the activation status. Barely 20 lines of code.

[grondag]

My point was that the current spec is not only easy to use as a mod wanting to change the status but also as one wanting to let other mods change your status by just acting as a renderer (in the narrow sense of this spec where a renderer is just some thing that influences how stuff gets rendered. not in the Fabric Rendering API sense where there can only be one).
You just ask FabricLoader for all frex_flawless_frames entrypoints, call each of them once with your setter and then they will call you whenever they wish to change the activation status. Barely 20 lines of code.

I see merit in what you are suggesting. To make sure I understand, let me restate it:

Any mod can provide this endpoint, and if it does it becomes a listener for all mods that want to control activation of the feature. That part doesn't create any new work for anyone.

A mod that wants to control this feature must be prepared to receive multiple consumers and when it wants to activate or deactivate the feature it must call all of them, or else different mods that depend on the feature will be out of sync.

This second part is a small but new demand on mods that want to control activation. Even if it is well documented, it's almost inevitable someone will miss it and just stash the first consumer they get and/or assume the API handles aggregation for them. Even so, it will probably be relatively obvious when this happens and it shouldn't be the norm - mods that do this sort of thing are generally already complicated and the authors know to be careful.

Is that the idea?

[Johni0702]

Yup, that's exactly what I was thinking.

[grondag]

Documentation of this feature has been updated to allow for multiple providers and consumers. This change is documentation-only.

https://github.com/grondag/frex/blob/1.17/src/main/java/grondag/frex/api/config/FlawlessFrames.java