fix: font not rendered correctly on init #8002
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| /** | ||
| * Random integer regenerated each scene update. | ||
| * | ||
| * Does not relate to elements versions, it's only a renderer | ||
| * cache-invalidation nonce at the moment. | ||
| */ | ||
| private sceneNonce: number | undefined; |
There was a problem hiding this comment.
versionNonce previously, while not inherently bad, could suggest dependency to elements version
|
|
||
| informMutation() { | ||
| this.versionNonce = randomInteger(); | ||
| triggerUpdate() { |
There was a problem hiding this comment.
informMutation() was a bit unclear and incorrect as the it didn't have to relate to mutation, and we generally just care about notifying that an update to the scene happened
| ); | ||
|
|
||
| private onSceneUpdated = () => { | ||
| private triggerRender = () => { |
There was a problem hiding this comment.
onSceneUpdated() was a bad name for something that only cares about triggering a render
| this.scene.mapElements((element) => { | ||
| if (isTextElement(element) && !isBoundToContainer(element)) { | ||
| ShapeCache.delete(element); | ||
| if (isTextElement(element)) { | ||
| didUpdate = true; | ||
| return newElementWith(element, { | ||
| ...refreshTextDimensions( | ||
| element, | ||
| getContainerElement(element, this.scene.getNonDeletedElementsMap()), | ||
| this.scene.getNonDeletedElementsMap(), | ||
| ), | ||
| }); | ||
| ShapeCache.delete(element); | ||
| return newElementWith(element, {}, true); | ||
| } |
There was a problem hiding this comment.
previously we were refreshing only non-containerized text elements because we were afraid (and still are) that reflowing a containerized text could end up changing the container dimensions across different browsers (clients).
Instead, I decided to not refresh text dimensions at all in this function, and just invalidate all text elements (including bound text) so as to force re-render.
@Mrazator but, we have to come up with a strategy for whether or when should we refresh text dimensions. Especially with respect to the coming virgil update which will have slightly different text dimensions. In that case, do we want to refresh dimensions, or is it ok not do it and leave it only after users edit the text later (if at all).
But, I'm actually not sure about this even in the current case, because it may happen the virgil doesn't load for some reason and fall back to a serif font with wildly different dimensions. Should we then refresh dimensions or leave bad bounding boxes? Tough call.
There was a problem hiding this comment.
As discussed on Discord, updating here:
- We will keep old Virgil for backwards compatibility.
- "it may happen the virgil doesn't load for some reason and fall back to a serif font with wildly different dimensions" - if we don't want to refresh dimensions because we are afraid of breaking container dimensions or the overall layout; then I would also not refresh dimensions at all if we are on a fallback font
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
The main issue was that we were not guaranteeing the text element object got a renewed reference, which then didn't update the scene, not renewing the
versionNonceor the elements array, so therenderStaticScenedidn't run (also the canvas cache would likely not get invalidated).