Title: Simplify `Sprite` API · Issue #83 · PAIR-code/megaplot · GitHub
Open Graph Title: Simplify `Sprite` API · Issue #83 · PAIR-code/megaplot
X Title: Simplify `Sprite` API · Issue #83 · PAIR-code/megaplot
Description: The objective of this issue is to simplify the Sprite API, making it easier to use and less likely to produce unexpeted or erroneous behavior. Sections: Summary of proposed Sprite API Background: Issues with the current Sprite API Walkth...
Open Graph Description: The objective of this issue is to simplify the Sprite API, making it easier to use and less likely to produce unexpeted or erroneous behavior. Sections: Summary of proposed Sprite API Background: I...
X Description: The objective of this issue is to simplify the Sprite API, making it easier to use and less likely to produce unexpeted or erroneous behavior. Sections: Summary of proposed Sprite API Background: I...
Opengraph URL: https://github.com/PAIR-code/megaplot/issues/83
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Simplify `Sprite` API","articleBody":"The objective of this issue is to simplify the Sprite API, making it easier to use and less likely to produce unexpeted or erroneous behavior.\r\n\r\nSections:\r\n\r\n - Summary of proposed Sprite API\r\n - Background: Issues with the current Sprite API\r\n - Walkthrough of proposed Sprite API\r\n - Discussion of callbacks vs Promises\r\n\r\n## Summary of proposed Sprite API\r\n\r\nSet attributes directly on Sprite object without having to schedule a callback (replaces the `.enter()` method):\r\n\r\n```ts\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.TransitionTimeMs = 250;\r\nsprite.PositionWorld = [0, 0];\r\nsprite.FillColor = [255, 127, 0, 1];\r\n```\r\n\r\nSet a singleton callback to be invoked after the next time values are flashed to the GPU (supersedes `.enter()`/`.update()` dynamic):\r\n\r\n```ts\r\nsprite.nextSync(() =\u003e {\r\n sprite.PositionWorld = getUpdatedPosition();\r\n});\r\n```\r\n\r\nSet a singleton callback to be invoked after the Sprite has finished its transition (supersedes `.exit()`):\r\n\r\n```ts\r\nsprite.transitionFinished(() =\u003e {\r\n sprite.detatch();\r\n});\r\n```\r\n\r\nA detatched sprite can be attached to the scene by calling the `.attach()` method. Attaching an attached sprite or detatching a detatched sprite has no effect. User can check `isAttached` to see its current state.\r\n\r\nIf there is insufficient capacity to accommodate attachcing a sprite, then it waits in an intermediate state until space is freed up. During this time, `isAttached` will be false, but `isWaiting` will be true.\r\n\r\nIf one or more attributes have been changed, but the data has not been flashed to the GPU, the `isDirty` property will be true.\r\n\r\n## Background: Issues with the current Sprite API\r\n\r\nIn this section, we'll walk through a fairly basic usage of the current Sprite API. This will demonstrate the interplay between memory management and timing which lead to unexpected and unintuitive behavior.\r\n\r\nThe current Sprite API requires users to use the `.enter()`, `.update()` and `.exit()` methods to schedule callbacks. Each callback receives a `SpriteView` object through which the user can specify attributes like `PositionWorld`, `FillColor`, etc. Here's an example:\r\n\r\n```ts\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.enter((s: SpriteView) =\u003e {\r\n s.PositionWorldX = 0;\r\n s.PositionWorldY = 0;\r\n s.FillColorR = 255;\r\n s.FillColorG = 127;\r\n s.FillColorB = 0;\r\n s.FillColorOpacity = 1;\r\n});\r\n```\r\n\r\nTo reduce the amount of code needed to set attribute values, the attributes offer destructuring assignment. The above code can be rewritten as follows:\r\n\r\n```ts\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.enter((s: SpriteView) =\u003e {\r\n s.PositionWorld = [0, 0];\r\n s.FillColor = [255, 127, 0, 1];\r\n});\r\n```\r\n\r\nThe downside of this approach is that there are now two, short-lived array objects which are created: `[0, 0]` and `[255, 127, 0, 1]`. For a single sprite, this is not a problem, but at scale, creating these kinds of objects causes noticeable performance degradation, and precipitates more frequent garbage collection.\r\n\r\nTo reduce this memory thrashing, users can move their objects outside of the callbacks for reuse, like this:\r\n\r\n```ts\r\nconst INITIAL_POSITION = [0, 0];\r\nconst INITIAL_COLOR = [255, 127, 0, 1];\r\n\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.enter((s: SpriteView) =\u003e {\r\n s.PositionWorld = INITIAL_POSITION;\r\n s.FillColor = INITIAL_COLOR;\r\n});\r\n```\r\n\r\nThis works, but the next problem is that the values are often dynamic in a visualization context. So rather than constants, the position and color would change over time:\r\n\r\n```ts\r\n// Dynamic properties change with visualization state.\r\nconst properties = {\r\n position: [0, 0],\r\n color: [255, 127, 0, 1],\r\n};\r\n\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.enter((s: SpriteView) =\u003e {\r\n // Set initial position and color.\r\n s.PositionWorld = properties.position;\r\n s.FillColor = properties.color;\r\n});\r\n\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\nsprite.update((s: SpriteView) =\u003e {\r\n // Set updated position and color.\r\n s.PositionWorld = properties.position;\r\n s.FillColor = properties.color;\r\n})\r\n```\r\n\r\nNotice here that we're using `.update()` for the second set of changes. Megaplot guarantees that the `.update()` callback will be called after the `.enter()` callback. But both callbacks are scheduled and invoked asynchronously.\r\n\r\nIn our running example, this means that the `.enter()` callback *may not have been run* by the time later that the `properties` values are changed. That is, the sprite may never have had its `PositionWorld` set to `[0, 0]` and its `FillColor` set to `[255, 127, 0, 1]`.\r\n\r\nTo solve this problem, the API user must explicitly capture the intended values for each callback. This can be done in any of several different ways. The following example promotes the initial state back to constants.\r\n\r\n```ts\r\nconst INITIAL_POSITION = [0, 0];\r\nconst INITIAL_COLOR = [255, 127, 0, 1];\r\n\r\n// Dynamic properties to change with visualization state.\r\nconst properties = {\r\n position: INITIAL_POSITION,\r\n color: INITIAL_COLOR,\r\n};\r\n\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.enter((s: SpriteView) =\u003e {\r\n // Set initial position and color.\r\n s.PositionWorld = INITIAL_POSITION;\r\n s.FillColor = INITIAL_COLOR;\r\n});\r\n\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\nsprite.update((s: SpriteView) =\u003e {\r\n // Set updated position and color.\r\n s.PositionWorld = properties.position;\r\n s.FillColor = properties.color;\r\n})\r\n```\r\n\r\nOverall, the behavior and subtle interaction between memory and timing makes the existing Sprite API difficult to work with and reason about.\r\n\r\n## Walkthrough of proposed Sprite API\r\n\r\nThe proposed Sprite API would be much simpler. Rather than requiring the API user to make changes inside of callbacks, the user sets properties directly on the sprite object:\r\n\r\n```ts\r\nconst sprite = scene.createSprite();\r\n\r\nsprite.PositionWorld = [0, 0];\r\nsprite.FillColor = [255, 127, 0, 1];\r\n```\r\n\r\nThis solves the problem of unexpected late execution. As soon as the setter is called, the values are read and stashed for eventual rendering. This is illutrated by adding a second instance of setting the values: \r\n\r\n```ts\r\n// Dynamic properties change with visualization state.\r\nconst properties = {\r\n position: [0, 0],\r\n color: [255, 127, 0, 1],\r\n};\r\n\r\nconst sprite = scene.createSprite();\r\n\r\n// This runs immediately, capturing the attribute values.\r\nsprite.PositionWorld = properties.position;\r\nsprite.FillColor = properties.color;\r\n\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\n// Set updated position and color.\r\nsprite.PositionWorld = properties.position;\r\nsprite.FillColor = properties.color;\r\n```\r\n\r\nWhile this solves the problem of unexpected late execution, there's another issue. Rendering is necessarily asynchronous, so it may be some time between when the attributes are set and the sprite is actually drawn to the screen. So while the sprite had its position and color set right away, it may have never rendered in that state before having its values updated.\r\n\r\nTo solve this issue, the API user needs to delay performing the update until after the values have been flashed to the GPU (at the earliest). We can make this possible by offering a `nextSync()` method, which takes a callback to invoke after the next time the values are flashed:\r\n\r\n```ts\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\nsprite.nextSync(() =\u003e {\r\n // Set updated position and color.\r\n sprite.PositionWorld = properties.position;\r\n sprite.FillColor = properties.color;\r\n})\r\n```\r\n\r\nNOTE: The callback provided to `nextSync()` could be invoked as soon as the next frame, but will *not* be called immediately in the same execution pass. As a general principle, an API that takes a callback should either always execute the callback immediately, or always wait for a future turn of the event loop. API methods that differentially delay execution are difficult to reason about and cause hard to predict behavior. For this reason, `nextSync()` must always delay invoking the callback, even if the sprite's `isDirty` flag currently reads false.\r\n\r\nRather than waiting simply for the next sync, which will often be the next frame, the API user may want to wait until the sprite has finished its transition. A method like `transitionFinished()` could make this easier:\r\n\r\n```ts\r\nconst TRANSITION_TIME_MS = 250;\r\n\r\n// Dynamic properties change with visualization state.\r\nconst properties = {\r\n position: [0, 0],\r\n color: [255, 127, 0, 1],\r\n};\r\n\r\nconst sprite = scene.createSprite();\r\n\r\n// This runs immediately, capturing the attribute values.\r\nsprite.TransitionTimeMs = TRANSITION_TIME_MS;\r\nsprite.PositionWorld = properties.position;\r\nsprite.FillColor = properties.color;\r\n\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\n// Set updated position and color.\r\nsprite.transitionFinished(() =\u003e {\r\n sprite.PositionWorld = properties.position;\r\n sprite.FillColor = properties.color;\r\n});\r\n```\r\n\r\n`transitionFinished()` enables chaining for more complex animations:\r\n\r\n```ts\r\nsprite.TransitionTimeMs = 250;\r\nsprite.FillColor = d3.color('red');\r\nsprite.transitionFinished(() =\u003e {\r\n sprite.FillColor = d3.color('orange');\r\n sprite.transitionFinished(() =\u003e {\r\n sprite.FillColor = d3.color('yellow');\r\n sprite.transitionFinished(() =\u003e {\r\n sprite.FillColor = d3.color('green');\r\n }); \r\n }); \r\n});\r\n```\r\n\r\nOne could be tempted to unroll these nested callbacks with Promises:\r\n\r\n```ts\r\n// NOT RECOMMENDED!\r\nsprite.TransitionTimeMs = 250;\r\nsprite.FillColor = d3.color('red');\r\nawait new Promise((r) =\u003e sprite.transitionFinished(r));\r\nsprite.FillColor = d3.color('orange');\r\nawait new Promise((r) =\u003e sprite.transitionFinished(r));\r\nsprite.FillColor = d3.color('yellow');\r\nawait new Promise((r) =\u003e sprite.transitionFinished(r));\r\nsprite.FillColor = d3.color('green');\r\n```\r\n\r\nWhile this approach does reduce the nesting observed in the nested callbacks, it introduces the possibility of forever-suspended functions. Each call to `transitionFinished()` supersedes the previous, overwriting the callback previously set (if any). So if, say, the above code was suspended on the first `await`, waiting for the sprite to finish transitioning to red, and some other callback calls `transitionFinished()`, then this function's execution context will hang forever.\r\n\r\nMegaplot could overcome this by returning Promises which are rejected when superceded. In that case, the API user should surround the transition chain with `try`/`catch` to handle the rejected Promise:\r\n\r\n```ts\r\n// NOT RECOMMENDED!\r\ntry {\r\n sprite.TransitionTimeMs = 250;\r\n sprite.FillColor = d3.color('red');\r\n await sprite.transitionFinished();\r\n sprite.FillColor = d3.color('orange');\r\n await sprite.transitionFinished();\r\n sprite.FillColor = d3.color('yellow');\r\n await sprite.transitionFinished();\r\n sprite.FillColor = d3.color('green');\r\n} catch (err) {\r\n // Transition superceded by later call!\r\n}\r\n```\r\n\r\nThe downside to providing this kind of aid is that it encourages writing `async` functions, which, in the interactive data visualization context, may make code more difficult to reason about. A lot can happen in between frames.\r\n\r\nFor more discussion of the tradeoffs between a callback-based and a Promise-based API, see the following section.\r\n\r\n## Discussion of callbacks vs Promises\r\n\r\nAs a design alternative, rather than taking callbacks (or perahps in addition), methods like `nextSync()` could return Promises. In the case of `nextSync()`, the returned Promise is fulfilled the next time values are flashed. For example:\r\n\r\n```ts\r\n// Later, dynamically...\r\n\r\nproperties.position = getUpdatedPosition();\r\nproperties.color = getUpdatedColor();\r\n\r\nawait sprite.nextSync();\r\n\r\n// Set updated position and color.\r\nsprite.PositionWorld = properties.position;\r\nsprite.FillColor = properties.color;\r\n```\r\n\r\nThere are several downsides to this approach. The first is that creating a Promise requires allocating a potentially short-lived object.\r\n\r\nAnother downside is that promises encourage the recipient to write `async` functions, which, in this context, may have lingering unpredictable behavior. A lot can happen in between frames.\r\n\r\nBut perhaps the best reason to prefer a callback API to a Promise one is that only a single callback function will be saved, while any number of async functions could be hanging on the same Promise.\r\n\r\nFor example, consider this code, which attempts to update the visualzitaion based on an end-user triggered event:\r\n\r\n```ts\r\ncontainerElement.addEventListener('mousemove', async () =\u003e {\r\n properties.position = getUpdatedPosition();\r\n properties.color = getUpdatedColor();\r\n\r\n await sprite.transitionFinished(); \r\n\r\n sprite.PositionWorld = properties.position;\r\n sprite.FillColor = properties.color;\r\n});\r\n```\r\n\r\nSince `mousemove` events can be prolific, this implementation could spawn many hanging `async` functions all waiting for the sprite to finish its last transition. This style would exacerbate memory thrashing if the post-await code does any allocation. Consider:\r\n\r\n```ts\r\ncontainerElement.addEventListener('mousemove', async () =\u003e {\r\n await sprite.transitionFinished(); \r\n sprite.FillColor = d3.color(getComputedColor());\r\n});\r\n```\r\n\r\n`d3.color()` allocates an object with `r`, `g`, `b` and `opacity` properties. So this seemingly innocuous, Promise-based code suspends a number of callback handles, each of which, when unsuspended, allocates a short-lived object.\r\n\r\nTo mitigate the potential for many hanging methods, it would be prudent to reject the previous Promise when a method like `transitionFinished()` is called again. In that case, the API user would want to surround the `await` with a `try`/`catch` block:\r\n\r\n```ts\r\ncontainerElement.addEventListener('mousemove', async () =\u003e {\r\n try {\r\n await sprite.transitionFinished(); \r\n sprite.FillColor = d3.color(getComputedColor());\r\n } catch (err) {\r\n // Probably superseded. Check err for details.\r\n }\r\n});\r\n```\r\n\r\nCompare that to the equivalent, callback-based implementation:\r\n\r\n```ts\r\ncontainerElement.addEventListener('mousemove', () =\u003e {\r\n sprite.transitionFinished(() =\u003e {\r\n sprite.FillColor = d3.color(getComputedColor());\r\n }); \r\n});\r\n```\r\n\r\nBecause of the additional complexity of the `try`/`catch` in the Promise-based code, the level of indentation here is identical. Plus, this code better handles a barrage of `mousemove` events. Each time `sprite.transitionFinished()` is called, the callback passed in supplants the previous callback, without triggering a `try`/`catch` flow.\r\n\r\nStrictly speaking, these two APIs are not mutually exclusive. The `transitionFinished()` implementation could return a Promise only when called without a callback. This would mitigate the creation of short-lived objects.\r\n\r\nHowever, since it's always possible to add the Promise-based features on later, for an initial implementation, the callback implementation is sufficient.\r\n","author":{"url":"https://github.com/jimbojw","@type":"Person","name":"jimbojw"},"datePublished":"2022-10-26T15:12:43.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":4},"url":"https://github.com/83/megaplot/issues/83"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:d9608224-f515-d3ab-cf92-5e770da22a86 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | E7AC:3DD3FA:154E1C4:1D65FBE:6A4DF1CC |
| html-safe-nonce | 129dbe4a590431eec7f5203a8ca9cf5e109badabb027da6e4eb2654da94710d4 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJFN0FDOjNERDNGQToxNTRFMUM0OjFENjVGQkU6NkE0REYxQ0MiLCJ2aXNpdG9yX2lkIjoiMzUyMjg5MDY0OTM3MTAxMzU4MCIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | f364ccfdfe20e4e55e245a3752eb7181c1d7a942adbbd6686aacfd641dc54da3 |
| hovercard-subject-tag | issue:1424234487 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/PAIR-code/megaplot/83/issue_layout |
| twitter:image | https://opengraph.githubassets.com/69d4d44991893483d65976141d198f23f47bd9633597d4d309cca57ac5f38571/PAIR-code/megaplot/issues/83 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/69d4d44991893483d65976141d198f23f47bd9633597d4d309cca57ac5f38571/PAIR-code/megaplot/issues/83 |
| og:image:alt | The objective of this issue is to simplify the Sprite API, making it easier to use and less likely to produce unexpeted or erroneous behavior. Sections: Summary of proposed Sprite API Background: I... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | jimbojw |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5818716c93c6a2925b815402541a32814e43a7b1261c322b0c2df75224289566 |
| turbo-cache-control | no-preview |
| go-import | github.com/PAIR-code/megaplot git https://github.com/PAIR-code/megaplot.git |
| octolytics-dimension-user_id | 29804435 |
| octolytics-dimension-user_login | PAIR-code |
| octolytics-dimension-repository_id | 451893434 |
| octolytics-dimension-repository_nwo | PAIR-code/megaplot |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 451893434 |
| octolytics-dimension-repository_network_root_nwo | PAIR-code/megaplot |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | f4bb89367ca678f057d79b1abc45d6675b1bd5b2 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width