Client gametest screenshot changes (#4329)

* Instant screenshots, with counter and more configurability

* Force consistent window size across all systems and stretch framebuffer to fit the physical window

* Docs

* Wait ticks appropriately for the screenshots in the gametest test

* Should -> must

* Fix window resizing for different display scales

* Fix framebuffer size not being changed at all

* Add test for window resizing

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/api/client/gametest/v1/ClientGameTestContext.java

@@ -111,19 +111,20 @@ public interface ClientGameTestContext {
 	boolean tryClickScreenButton(String translationKey);
 
 	/**
-	 * Takes a screenshot after waiting 1 tick (for a frame to render) and saves it in the screenshots directory.
+	 * Takes a screenshot and saves it in the screenshots directory.
 	 *
 	 * @param name The name of the screenshot
+	 * @return The {@link Path} to the screenshot
 	 */
 	Path takeScreenshot(String name);
 
 	/**
-	 * Takes a screnshot after waiting {@code delay} ticks and saves it in the screenshots directory.
+	 * Takes a screenshot with the given options.
 	 *
-	 * @param name The name of the screenshot
-	 * @param delay The delay in ticks before taking the screenshot
+	 * @param options The {@link TestScreenshotOptions} to take the screenshot with
+	 * @return The {@link Path} to the screenshot
 	 */
-	Path takeScreenshot(String name, int delay);
+	Path takeScreenshot(TestScreenshotOptions options);
 
 	/**
 	 * Gets the input handler used to simulate inputs to the client.

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/api/client/gametest/v1/TestInput.java

@@ -325,4 +325,14 @@ public interface TestInput {
 	 * @see #setCursorPos(double, double)
 	 */
 	void moveCursor(double deltaX, double deltaY);
+
+	/**
+	 * Resizes the window to match the given size. Also attempts to resize the physical window, but whether the physical
+	 * window was successfully resized or not, the window size accessible by the game will always be changed to the
+	 * value specified, causing widget layouts and screenshots to work as expected.
+	 *
+	 * @param width The new window width
+	 * @param height The new window height
+	 */
+	void resizeWindow(int width, int height);
 }

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/api/client/gametest/v1/TestScreenshotOptions.java

@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.api.client.gametest.v1;
+
+import java.nio.file.Path;
+
+import com.google.common.base.Preconditions;
+import org.jetbrains.annotations.ApiStatus;
+
+import net.fabricmc.fabric.impl.client.gametest.TestScreenshotOptionsImpl;
+
+/**
+ * Options to customize a screenshot.
+ */
+@ApiStatus.NonExtendable
+public interface TestScreenshotOptions {
+	/**
+	 * Creates a {@link TestScreenshotOptions} with the given screenshot name.
+	 *
+	 * @param name The name of the screenshot
+	 * @return The new screenshot options instance
+	 */
+	static TestScreenshotOptions of(String name) {
+		Preconditions.checkNotNull(name, "name");
+		return new TestScreenshotOptionsImpl(name);
+	}
+
+	/**
+	 * By default, screenshot file names will be prefixed by a counter so that the screenshots appear in sequence in the
+	 * screenshots directory. Use this method to disable this behavior.
+	 *
+	 * @return This screenshot options instance
+	 */
+	TestScreenshotOptions disableCounterPrefix();
+
+	/**
+	 * Changes the tick delta to take this screenshot with. Tick delta controls interpolation between the previous tick and the
+	 * current tick to make objects appear to move more smoothly when there are multiple frames in a tick. Defaults to
+	 * {@code 1}, which renders all objects as their appear in the current tick.
+	 *
+	 * @param tickDelta The tick delta to take this screenshot with
+	 * @return This screenshot options instance
+	 */
+	TestScreenshotOptions withTickDelta(float tickDelta);
+
+	/**
+	 * Changes the resolution of the screenshot, which defaults to the resolution of the Minecraft window.
+	 *
+	 * @param width The width of the screenshot
+	 * @param height The height of the screenshot
+	 * @return This screenshot options instance
+	 */
+	TestScreenshotOptions withSize(int width, int height);
+
+	/**
+	 * Changes the directory in which this screenshot is saved, which defaults to the {@code screenshots} directory in
+	 * the game's run directory.
+	 *
+	 * @param destinationDir The directory in which to save the screenshot
+	 * @return This screenshot options instance
+	 */
+	TestScreenshotOptions withDestinationDir(Path destinationDir);
+}

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/ClientGameTestContextImpl.java

@@ -16,9 +16,12 @@
 
 package net.fabricmc.fabric.impl.client.gametest;
 
+import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
@@ -41,16 +44,19 @@
 import net.minecraft.client.option.CloudRenderMode;
 import net.minecraft.client.option.GameOptions;
 import net.minecraft.client.option.SimpleOption;
+import net.minecraft.client.texture.NativeImage;
 import net.minecraft.client.tutorial.TutorialStep;
 import net.minecraft.client.util.ScreenshotRecorder;
 import net.minecraft.sound.SoundCategory;
 import net.minecraft.text.Text;
 import net.minecraft.util.Nullables;
 
 import net.fabricmc.fabric.api.client.gametest.v1.ClientGameTestContext;
+import net.fabricmc.fabric.api.client.gametest.v1.TestScreenshotOptions;
 import net.fabricmc.fabric.api.client.gametest.v1.TestWorldBuilder;
 import net.fabricmc.fabric.mixin.client.gametest.CyclingButtonWidgetAccessor;
 import net.fabricmc.fabric.mixin.client.gametest.GameOptionsAccessor;
+import net.fabricmc.fabric.mixin.client.gametest.RenderTickCounterConstantAccessor;
 import net.fabricmc.fabric.mixin.client.gametest.ScreenAccessor;
 import net.fabricmc.loader.api.FabricLoader;
 
@@ -262,22 +268,56 @@ private static boolean pressMatchingButton(ClickableWidget widget, String text)
 	public Path takeScreenshot(String name) {
 		ThreadingImpl.checkOnGametestThread("takeScreenshot");
 		Preconditions.checkNotNull(name, "name");
-		return takeScreenshot(name, 1);
+		return takeScreenshot(TestScreenshotOptions.of(name));
 	}
 
 	@Override
-	public Path takeScreenshot(String name, int delay) {
+	public Path takeScreenshot(TestScreenshotOptions options) {
 		ThreadingImpl.checkOnGametestThread("takeScreenshot");
-		Preconditions.checkNotNull(name, "name");
-		Preconditions.checkArgument(delay >= 0, "delay cannot be negative");
+		Preconditions.checkNotNull(options, "options");
 
-		waitTicks(delay);
-		runOnClient(client -> {
-			ScreenshotRecorder.saveScreenshot(FabricLoader.getInstance().getGameDir().toFile(), name + ".png", client.getFramebuffer(), (message) -> {
-			});
-		});
+		TestScreenshotOptionsImpl optionsImpl = (TestScreenshotOptionsImpl) options;
+		return computeOnClient(client -> {
+			int prevWidth = client.getWindow().getFramebufferWidth();
+			int prevHeight = client.getWindow().getFramebufferHeight();
+
+			if (optionsImpl.size != null) {
+				client.getWindow().setFramebufferWidth(optionsImpl.size.x);
+				client.getWindow().setFramebufferHeight(optionsImpl.size.y);
+				client.getFramebuffer().resize(optionsImpl.size.x, optionsImpl.size.y);
+			}
+
+			try {
+				client.gameRenderer.render(RenderTickCounterConstantAccessor.create(optionsImpl.tickDelta), true);
+
+				// The vanilla panorama screenshot code has a Thread.sleep(10) here, is this needed?
+
+				Path destinationDir = Objects.requireNonNullElseGet(optionsImpl.destinationDir, () -> FabricLoader.getInstance().getGameDir().resolve("screenshots"));
 
-		return FabricLoader.getInstance().getGameDir().resolve("screenshots").resolve(name + ".png");
+				try {
+					Files.createDirectories(destinationDir);
+				} catch (IOException e) {
+					throw new AssertionError("Failed to create screenshots directory", e);
+				}
+
+				String counterPrefix = optionsImpl.counterPrefix ? "%04d_".formatted(ClientGameTestImpl.screenshotCounter++) : "";
+				Path screenshotFile = destinationDir.resolve(counterPrefix + optionsImpl.name + ".png");
+
+				try (NativeImage screenshot = ScreenshotRecorder.takeScreenshot(client.getFramebuffer())) {
+					screenshot.writeTo(screenshotFile);
+				} catch (IOException e) {
+					throw new AssertionError("Failed to write screenshot file", e);
+				}
+
+				return screenshotFile;
+			} finally {
+				if (optionsImpl.size != null) {
+					client.getWindow().setFramebufferWidth(prevWidth);
+					client.getWindow().setFramebufferHeight(prevHeight);
+					client.getFramebuffer().resize(prevWidth, prevHeight);
+				}
+			}
+		});
 	}
 
 	@Override

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/ClientGameTestImpl.java

@@ -31,6 +31,7 @@
 
 public final class ClientGameTestImpl {
 	public static final Logger LOGGER = LoggerFactory.getLogger("fabric-client-gametest-api-v1");
+	public static int screenshotCounter = 0;
 
 	private ClientGameTestImpl() {
 	}

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/FabricClientGameTestRunner.java

@@ -38,17 +38,24 @@ public static void start() {
 			ClientGameTestContextImpl context = new ClientGameTestContextImpl();
 
 			for (FabricClientGameTest gameTest : gameTests) {
-				context.restoreDefaultGameOptions();
+				setupInitialGameTestState(context);
 
 				gameTest.runTest(context);
 
-				context.getInput().clearKeysDown();
-				checkFinalGameTestState(context, gameTest.getClass().getName());
+				setupAndCheckFinalGameTestState(context, gameTest.getClass().getName());
 			}
 		});
 	}
 
-	private static void checkFinalGameTestState(ClientGameTestContext context, String testClassName) {
+	private static void setupInitialGameTestState(ClientGameTestContext context) {
+		context.restoreDefaultGameOptions();
+	}
+
+	private static void setupAndCheckFinalGameTestState(ClientGameTestContextImpl context, String testClassName) {
+		context.getInput().clearKeysDown();
+		context.runOnClient(client -> ((WindowHooks) (Object) client.getWindow()).fabric_resetSize());
+		context.getInput().setCursorPos(context.computeOnClient(client -> client.getWindow().getWidth()) * 0.5, context.computeOnClient(client -> client.getWindow().getHeight()) * 0.5);
+
 		if (ThreadingImpl.isServerRunning) {
 			throw new AssertionError("Client gametest %s finished while a server is still running".formatted(testClassName));
 		}

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/TestInputImpl.java

@@ -321,6 +321,15 @@ public void moveCursor(double deltaX, double deltaY) {
 		});
 	}
 
+	@Override
+	public void resizeWindow(int width, int height) {
+		ThreadingImpl.checkOnGametestThread("resizeWindow");
+		Preconditions.checkArgument(width > 0, "width must be positive");
+		Preconditions.checkArgument(height > 0, "height must be positive");
+
+		context.runOnClient(client -> ((WindowHooks) (Object) client.getWindow()).fabric_resize(width, height));
+	}
+
 	private static InputUtil.Key getBoundKey(KeyBinding keyBinding, String action) {
 		InputUtil.Key boundKey = ((KeyBindingAccessor) keyBinding).getBoundKey();
 

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/TestScreenshotOptionsImpl.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.impl.client.gametest;
+
+import java.nio.file.Path;
+
+import com.google.common.base.Preconditions;
+import org.jetbrains.annotations.Nullable;
+import org.joml.Vector2i;
+
+import net.fabricmc.fabric.api.client.gametest.v1.TestScreenshotOptions;
+
+public class TestScreenshotOptionsImpl implements TestScreenshotOptions {
+	public final String name;
+	public boolean counterPrefix = true;
+	public float tickDelta = 1;
+	@Nullable
+	public Vector2i size;
+	@Nullable
+	public Path destinationDir;
+
+	public TestScreenshotOptionsImpl(String name) {
+		this.name = name;
+	}
+
+	@Override
+	public TestScreenshotOptions disableCounterPrefix() {
+		counterPrefix = false;
+		return this;
+	}
+
+	@Override
+	public TestScreenshotOptions withTickDelta(float tickDelta) {
+		Preconditions.checkArgument(tickDelta >= 0 && tickDelta <= 1, "tickDelta must be between 0 and 1");
+
+		this.tickDelta = tickDelta;
+		return this;
+	}
+
+	@Override
+	public TestScreenshotOptions withSize(int width, int height) {
+		Preconditions.checkArgument(width > 0, "width must be positive");
+		Preconditions.checkArgument(height > 0, "height must be positive");
+
+		this.size = new Vector2i(width, height);
+		return this;
+	}
+
+	@Override
+	public TestScreenshotOptions withDestinationDir(Path destinationDir) {
+		Preconditions.checkNotNull(destinationDir, "destinationDir");
+
+		this.destinationDir = destinationDir;
+		return this;
+	}
+}

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/WindowHooks.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.impl.client.gametest;
+
+public interface WindowHooks {
+	int fabric_getRealWidth();
+	int fabric_getRealHeight();
+	int fabric_getRealFramebufferWidth();
+	int fabric_getRealFramebufferHeight();
+	void fabric_resetSize();
+	void fabric_resize(int width, int height);
+}