From 797a71695fd1d2d5d74dae9972221a4405071047 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 26 Aug 2023 09:37:08 -0700 Subject: [PATCH 1/2] Improve texture atlas utilization - Changed the threshold at which pages start to merge from MAX/2 to MAX. - If a glyph is about to create a new page that will be merged, check if the existing row fits when ignoring the row pixel threshold. This will improve texture utilization by using the available space before the page is merged and becomes static. --- src/browser/renderer/shared/TextureAtlas.ts | 34 +++++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index c7b85a7e82..4444d8f605 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -151,7 +151,7 @@ export class TextureAtlas implements ITextureAtlas { // microtask to ensure it does not interrupt textures that will be rendered in the current // animation frame which would result in blank rendered areas. This is actually not that // expensive relative to drawing the glyphs, so there is no need to wait for an idle callback. - if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages / 2)) { + if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages)) { queueMicrotask(() => { // Find the set of the largest 4 images, below the maximum size, with the highest // percentages used @@ -756,13 +756,13 @@ export class TextureAtlas implements ITextureAtlas { } } - // Create a new one if too much vertical space would be wasted or there is not enough room + // Create a new page if too much vertical space would be wasted or there is not enough room // left in the page. The previous active row will become fixed in the process as it now has a // fixed height if (activeRow.y + rasterizedGlyph.size.y >= activePage.canvas.height || activeRow.height > rasterizedGlyph.size.y + Constants.ROW_PIXEL_THRESHOLD) { // Create the new fixed height row, creating a new page if there isn't enough room on the // current page - let wasNewPageCreated = false; + let wasPageAndRowFound = false; if (activePage.currentRow.y + activePage.currentRow.height + rasterizedGlyph.size.y >= activePage.canvas.height) { // Find the first page with room to create the new row on let candidatePage: AtlasPage | undefined; @@ -775,15 +775,29 @@ export class TextureAtlas implements ITextureAtlas { if (candidatePage) { activePage = candidatePage; } else { - // Create a new page if there is no room - const newPage = this._createNewPage(); - activePage = newPage; - activeRow = newPage.currentRow; - activeRow.height = rasterizedGlyph.size.y; - wasNewPageCreated = true; + // Before creating a new atlas page that would trigger a page merge, check if the + // current active row is sufficient when ignoring the ROW_PIXEL_THRESHOLD. This will + // improve texture utilization by using the available space before the page is merged + // and becomes static. + if ( + TextureAtlas.maxAtlasPages === this._pages.length && + activeRow.y + rasterizedGlyph.size.y <= activePage.canvas.height && + activeRow.height >= rasterizedGlyph.size.y && + activeRow.x + rasterizedGlyph.size.x <= activePage.canvas.width + ) { + // activePage and activeRow is already valid + wasPageAndRowFound = true; + } else { + // Create a new page if there is no room + const newPage = this._createNewPage(); + activePage = newPage; + activeRow = newPage.currentRow; + activeRow.height = rasterizedGlyph.size.y; + wasPageAndRowFound = true; + } } } - if (!wasNewPageCreated) { + if (!wasPageAndRowFound) { // Fix the current row as the new row is being added below if (activePage.currentRow.height > 0) { activePage.fixedRows.push(activePage.currentRow); From aba16687b0723a6cc5f13f921dd93011ca69c050 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sat, 26 Aug 2023 10:23:09 -0700 Subject: [PATCH 2/2] Fix high pressure texture atlas corruption The corruption was caused by confusion with page indexes when merging pages. The fix is to simplify it by deleting all pages and then just adding the new page. Fixes #4534 Fixes #4351 --- src/browser/renderer/shared/TextureAtlas.ts | 75 ++++++++++----------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/browser/renderer/shared/TextureAtlas.ts b/src/browser/renderer/shared/TextureAtlas.ts index 4444d8f605..dd05957408 100644 --- a/src/browser/renderer/shared/TextureAtlas.ts +++ b/src/browser/renderer/shared/TextureAtlas.ts @@ -152,49 +152,47 @@ export class TextureAtlas implements ITextureAtlas { // animation frame which would result in blank rendered areas. This is actually not that // expensive relative to drawing the glyphs, so there is no need to wait for an idle callback. if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages)) { - queueMicrotask(() => { - // Find the set of the largest 4 images, below the maximum size, with the highest - // percentages used - const pagesBySize = this._pages.filter(e => { - return e.canvas.width * 2 <= (TextureAtlas.maxTextureSize || Constants.FORCED_MAX_TEXTURE_SIZE); - }).sort((a, b) => { - if (b.canvas.width !== a.canvas.width) { - return b.canvas.width - a.canvas.width; - } - return b.percentageUsed - a.percentageUsed; - }); - let sameSizeI = -1; - let size = 0; - for (let i = 0; i < pagesBySize.length; i++) { - if (pagesBySize[i].canvas.width !== size) { - sameSizeI = i; - size = pagesBySize[i].canvas.width; - } else if (i - sameSizeI === 3) { - break; - } + // Find the set of the largest 4 images, below the maximum size, with the highest + // percentages used + const pagesBySize = this._pages.filter(e => { + return e.canvas.width * 2 <= (TextureAtlas.maxTextureSize || Constants.FORCED_MAX_TEXTURE_SIZE); + }).sort((a, b) => { + if (b.canvas.width !== a.canvas.width) { + return b.canvas.width - a.canvas.width; + } + return b.percentageUsed - a.percentageUsed; + }); + let sameSizeI = -1; + let size = 0; + for (let i = 0; i < pagesBySize.length; i++) { + if (pagesBySize[i].canvas.width !== size) { + sameSizeI = i; + size = pagesBySize[i].canvas.width; + } else if (i - sameSizeI === 3) { + break; } + } - // Gather details of the merge - const mergingPages = pagesBySize.slice(sameSizeI, sameSizeI + 4); - const sortedMergingPagesIndexes = mergingPages.map(e => e.glyphs[0].texturePage).sort((a, b) => a > b ? 1 : -1); - const mergedPageIndex = sortedMergingPagesIndexes[0]; + // Gather details of the merge + const mergingPages = pagesBySize.slice(sameSizeI, sameSizeI + 4); + const sortedMergingPagesIndexes = mergingPages.map(e => e.glyphs[0].texturePage).sort((a, b) => a > b ? 1 : -1); + const mergedPageIndex = this.pages.length - mergingPages.length; - // Merge into the new page - const mergedPage = this._mergePages(mergingPages, mergedPageIndex); - mergedPage.version++; + // Merge into the new page + const mergedPage = this._mergePages(mergingPages, mergedPageIndex); + mergedPage.version++; - // Replace the first _merging_ page with the _merged_ page - this._pages[mergedPageIndex] = mergedPage; + // Delete the pages, shifting glyph texture pages as needed + for (let i = sortedMergingPagesIndexes.length - 1; i >= 0; i--) { + this._deletePage(sortedMergingPagesIndexes[i]); + } - // Delete the other 3 pages, shifting glyph texture pages as needed - for (let i = sortedMergingPagesIndexes.length - 1; i >= 1; i--) { - this._deletePage(sortedMergingPagesIndexes[i]); - } + // Add the new merged page to the end + this.pages.push(mergedPage); - // Request the model to be cleared to refresh all texture pages. - this._requestClearModel = true; - this._onAddTextureAtlasCanvas.fire(mergedPage.canvas); - }); + // Request the model to be cleared to refresh all texture pages. + this._requestClearModel = true; + this._onAddTextureAtlasCanvas.fire(mergedPage.canvas); } // All new atlas pages are created small as they are highly dynamic @@ -780,7 +778,8 @@ export class TextureAtlas implements ITextureAtlas { // improve texture utilization by using the available space before the page is merged // and becomes static. if ( - TextureAtlas.maxAtlasPages === this._pages.length && + TextureAtlas.maxAtlasPages && + this._pages.length >= TextureAtlas.maxAtlasPages && activeRow.y + rasterizedGlyph.size.y <= activePage.canvas.height && activeRow.height >= rasterizedGlyph.size.y && activeRow.x + rasterizedGlyph.size.x <= activePage.canvas.width