DEV-974: support different GIF sizes #26

torbensky · 2022-03-01T22:44:22Z

This PR updates the rendering to support any GIF size. Previously the GIF size was determined by the board size.

Now, GIF size parameters can be passed and the renderer will centre the board within the GIF. It will maximise the size of the board such that the board squares fill up as much of the width/height as they can without being clipped. This logic is able to handle non-square boards.

Three new API methods have been added:

GET /games/<GAME_ID>/<SIZE>.gif
GET /games/<GAME_ID>/frames/<FRAME>/SIZE>.gif
GET /games/<GAME_ID>/frames/<FRAME>.txt

Where <SIZE> is a path parameter expecting the format <WIDTH>x<HEIGHT>.

In order to maintain backwards-compatibility with the legacy GIF API methods, the old calculations are preserved for when a GIF size is not specified. That logic is GIF width = <board width> * 20 + <board border> * 2 and GIF height = <board height> * 20 + <board border> * 2. For example, a 11x11 board would default to creating a 224x224 GIF, and a 19x19 board defaults to creating a 384x384 GIF.

While the underlying logic is able to support any size, for practical reasons the API will only allow a discrete set of GIF sizes. This is because:

An upper bound is important to prevent massively large GIFs (the file size of GIFs grows quickly with resolution and so does the processing time required to generate them).
A lower bound prevents GIFs which are not visually useful (below a certain point there is not enough resolution to distinguish important game features).
Discrete size options ensures caching is effective and reduces unnecessary computations for resolutions which are not meaningfully different

Because boards can be of varying dimensions, the allowed options will be based on the board dimensions. Those dimensions are currently:

10 pixels per square (+ 4 pixels for border)
20 pixels per square (+ 4 pixels for border)
30 pixels per square (+ 4 pixels for border)
40 pixels per square (+ 4 pixels for border)

Examples:

sizes allowed for 11x11 board are: 114x114, 224x224, 334x334 and 444x444
sizes allowed for 19x19 board are: 194x194, 384x384, ~~574x574 and 764x764~~
- 574x574 and 764x764 would be disallowed by the 504x504 upper-limit for max resolution
etc...

Importantly, if use cases requiring other sizes are surfaced, these options can quickly be changed by simply changing the validation logic, since the underlying rendering handles any size.

Summary

Added new API methods which include parameters for GIF dimensions
Only a restricted subset of dimensions are allowed by the API
Board is centered when GIF size does not perfectly fit board dimensions
Within the core rendering logic, any GIF dimensions are supported, not just square ones

update GIF test for larger food size resulting from float math

torbensky · 2022-03-02T19:22:14Z

render/image.go

-func drawWatermark(dc *gg.Context) {
-	watermarkImage, err := media.GetWatermarkPNG(dc.Width()*2/3, dc.Height()*2/3)
+func drawWatermark(dc *boardContext) {
+


This was updated so it properly sizes the watermark according to the rendered board size, not just the size of the entire image (GIF). This ensures that the watermark stays within the board and that it doesn't get stretched in weird aspect ratios.

torbensky · 2022-03-03T00:25:52Z

http/handlers.go

+const maxGIFResolution = 504 * 504
+
+// allowedPixelsPerSquare is a list of resolutions that the API will allow.
+var allowedPixelsPerSquare = []int{10, 20, 30, 40}


So this is what I came up with for restricting the GIF resolutions to limited options. Size 20 is the same as what we used to have hard-coded.

I realised that custom games can have any board size, so the restrictions would need to adapt to the board. This way every board size can have a few options that fit it well.

In addition to these options, there is the upper-limit of 504x504, which supersedes these options.

So larger boards won't have all these options because they'd exceed the max resolution limit.

A 19x19 board would only have the options of 194x194 and 384x384.

robbles

Looks good, and I tested the new and old endpoints locally with no problems.
Only "regression" is the slight change in size of the food pellets on the legacy GIF endpoints, which seems to be captured in the image test in the PR.

robbles · 2022-03-07T21:18:50Z

go.mod

 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/sirupsen/logrus v1.8.1
 	github.com/stretchr/testify v1.7.0
+	goji.io v2.0.2+incompatible


Do we want to avoid the newer v3.0.0 version of Goji? Development on the project looks paused for a few years, are you thinking to use the latest stable version because of that?

I actually didn't notice this to be honest. This was the version that Go selected when I ran go get github.com/goji/goji

I updated to v3 - nice job noticing that :D

robbles · 2022-03-07T21:27:26Z

README.md

+  - **574x574** (30 pixels per board square) (**Disallowed** because it exceeds `504x504`)
+  - **764x764** (40 pixels per board square) (**Disallowed** because it exceeds `504x504`)
+- etc...
+


Since there's no common allowed board size, is the idea here that Play will need to do a calculation to generate the URL for a preset size in order to link the GIF for a game? e.g. we'd decide we want the "View GIF" button to link to the "30 pixels per board square" resolution, and generate the URL accordingly on the fly?

Good point. Do you think it would be better to have the parameters be the resolution per-square instead?

Good point. Do you think it would be better to have the dimension parameters be the resolution per-square instead? I feel like that would be better so Play wouldn't have to do those calculations.

I originally started with allowing any size. Then after some discussion, it seemed that limited options were a good idea, but there wasn't a good way to do this with static sizes, so I came up with this.

I'm not sure which is better, TBH. There's something about having the actual image size reflected in the URL that seems convenient. I don't think it's a big deal for play to include that calculation. Might be worth asking in #dev in case there's a use case I've missed here.

I brought this up with the team this morning, and the consensus was that we haven't fully worked out the use cases for different GIF sizes, so we can just ship as-is for now. We may end up revisiting the endpoint design with something like small/medium/large mapping to fixed pixels-per-square values, which I think won't be incompatible with the routing approach used here?

Sounds good Rob. Yeah I think the routing is quite flexible. It should be easy enough to adapt this to other parameters in the future.

torbensky · 2022-03-08T04:19:18Z

Looks good, and I tested the new and old endpoints locally with no problems.
Only "regression" is the slight change in size of the food pellets on the legacy GIF endpoints, which seems to be captured in the image test in the PR.

@robbles I changed the food size in this PR because I thought it didn't match with the Play UI. But maybe I got that wrong.

update: actually, it looks like it might be smaller than the Play board by just a wee bit. I think that might be due to rounding errors. Previously the Exporter code used integer math and divided the square size by 3 to get the food radius. Now it's using floating point math, so the circles are more exactly 1/3 the size of the square.

Does the Play board do integer rounding?

If so, I'm not sure which way should be considered correct, but I would normally assume the float math is better because we will have consistently sized food. The integer rounding will result in inconsistent food to square sizing.

robbles · 2022-03-08T15:51:04Z

I think the new food size is good - didn't mean to imply it was worse, just the only difference I was able to spot.

torbensky added 7 commits March 1, 2022 12:31

WIP: support variable GIF sizes

33649a0

POC of variable sizes working

5101270

convert to float math

9768a7d

fix: artifacts - use partial float/int math

c96d1a7

support legacy methods by calculating height/width

24d82cb

use consistent naming

69409db

center board when GIF size != board dimensions

08ef6f1

update GIF test for larger food size resulting from float math

torbensky added the enhancement New feature or request label Mar 1, 2022

torbensky changed the title ~~Dev 974 variable gif size 3~~ DEV-974: support different GIF sizes Mar 1, 2022

torbensky added 8 commits March 1, 2022 14:49

cleanup commented out code

ad0d765

center and scale watermark appropriately

a574975

basic lower/upper limits for GIF size

14910df

remove lower bound limit

3bd302d

implement predefined, allowable GIF resolutions

b37f343

cleanup some comments

4a25104

improve comment clarity

befd78f

remove dead code

82c53e2

torbensky commented Mar 2, 2022

View reviewed changes

improve test coverage

e4b8b6a

torbensky commented Mar 3, 2022

View reviewed changes

torbensky marked this pull request as ready for review March 3, 2022 00:32

torbensky requested review from robbles and bvanvugt as code owners March 3, 2022 00:32

torbensky added 6 commits March 4, 2022 15:19

switch to goji + switch to suffix style paths

0c166c9

add new ".txt" path for ascii frames

c1b869d

update readme and go mod

0994c5a

refactor common handler code

805d926

cleanup commented code

3bdcb88

adjust caller to be correct

836713b

robbles approved these changes Mar 7, 2022

View reviewed changes

update to goji.io v3

37befd7

torbensky closed this Mar 8, 2022

torbensky reopened this Mar 8, 2022

torbensky merged commit 093f9f6 into main Mar 8, 2022

torbensky deleted the dev-974-variable-gif-size-3 branch March 8, 2022 18:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEV-974: support different GIF sizes #26

DEV-974: support different GIF sizes #26

torbensky commented Mar 1, 2022 •

edited

Loading

torbensky Mar 2, 2022 •

edited

Loading

torbensky Mar 3, 2022 •

edited

Loading

torbensky Mar 3, 2022

robbles left a comment

robbles Mar 7, 2022

torbensky Mar 8, 2022

torbensky Mar 8, 2022

robbles Mar 7, 2022

torbensky Mar 8, 2022

torbensky Mar 8, 2022

robbles Mar 8, 2022

robbles Mar 8, 2022

torbensky Mar 8, 2022

torbensky commented Mar 8, 2022 •

edited

Loading

robbles commented Mar 8, 2022

DEV-974: support different GIF sizes #26

DEV-974: support different GIF sizes #26

Conversation

torbensky commented Mar 1, 2022 • edited Loading

torbensky Mar 2, 2022 • edited Loading

Choose a reason for hiding this comment

torbensky Mar 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robbles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torbensky commented Mar 8, 2022 • edited Loading

robbles commented Mar 8, 2022

torbensky commented Mar 1, 2022 •

edited

Loading

torbensky Mar 2, 2022 •

edited

Loading

torbensky Mar 3, 2022 •

edited

Loading

torbensky commented Mar 8, 2022 •

edited

Loading