Skip to content

Commit

Permalink
Ignore shader uniform errors by default (backport #3754) (#3759)
Browse files Browse the repository at this point in the history
Co-authored-by: DStradley <48810710+DStradley@users.noreply.github.com>
  • Loading branch information
mergify[bot] and DStradley authored Jun 8, 2022
1 parent 8c2b6e1 commit adcd85d
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 12 deletions.
1 change: 1 addition & 0 deletions common/api/core-frontend.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -8945,6 +8945,7 @@ export namespace RenderSystem {
doIdleWork?: boolean;
dpiAwareLOD?: boolean;
dpiAwareViewports?: boolean;
errorOnMissingUniform?: boolean;
// @internal
filterMapDrapeTextures?: boolean;
// @internal
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-frontend",
"comment": "added errorOnMissingUniform render option",
"type": "none"
}
],
"packageName": "@itwin/core-frontend"
}
6 changes: 6 additions & 0 deletions core/frontend/src/render/RenderSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,12 @@ export namespace RenderSystem { // eslint-disable-line no-redeclare
*/
contextAttributes?: WebGLContextAttributes;

/** If true, will cause exception when a shader uniform is missing (usually optimized out), otherwise will only log these.
* Default value: false
* @public
*/
errorOnMissingUniform?: boolean;

/** If true, and the `WEBGL_debug_shaders` extension is available, accumulate debug information during shader compilation.
* This information can be accessed via `RenderSystemDebugControl.debugShaderFiles`.
* Default value: false
Expand Down
2 changes: 1 addition & 1 deletion core/frontend/src/render/webgl/ShaderProgram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class Uniform {
public compile(prog: ShaderProgram): boolean {
assert(!this.isValid);
if (undefined !== prog.glProgram) {
this._handle = UniformHandle.create(prog.glProgram, this._name);
this._handle = UniformHandle.create(prog, this._name);
}

return this.isValid;
Expand Down
25 changes: 18 additions & 7 deletions core/frontend/src/render/webgl/UniformHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
*/

import { assert } from "@itwin/core-bentley";
import { Logger } from "@itwin/core-bentley/lib/cjs/Logger";
import { FrontendLoggerCategory } from "../../FrontendLoggerCategory";
import { Matrix3, Matrix4 } from "./Matrix";
import { ShaderProgram } from "./ShaderProgram";
import { SyncToken } from "./Sync";
import { System } from "./System";

Expand All @@ -29,18 +32,26 @@ const enum DataType {// eslint-disable-line no-restricted-syntax
* @internal
*/
export class UniformHandle {
private readonly _location: WebGLUniformLocation;
private readonly _location: WebGLUniformLocation | null;
private _type: DataType = DataType.Undefined;
private readonly _data: number[] = [];
public syncToken?: SyncToken;

private constructor(location: WebGLUniformLocation) { this._location = location; }

public static create(program: WebGLProgram, name: string): UniformHandle {
const location = System.instance.context.getUniformLocation(program, name);
if (null === location)
throw new Error(`uniform ${name} not found.`);
private constructor(location: WebGLUniformLocation | null) { this._location = location; }

public static create(program: ShaderProgram, name: string): UniformHandle {
let location = null;
if (undefined !== program.glProgram) {
location = System.instance.context.getUniformLocation(program.glProgram, name);
}
if (null === location) {
const errMsg = `uniform ${name} not found in ${program.description}.`;
if (System.instance.options.errorOnMissingUniform) {
throw new Error(errMsg);
} else {
Logger.logError(FrontendLoggerCategory.Render, errMsg);
}
}
return new UniformHandle(location);
}

Expand Down
14 changes: 11 additions & 3 deletions core/frontend/src/test/render/webgl/Technique.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { System } from "../../../render/webgl/System";
import { Target } from "../../../render/webgl/Target";
import { TechniqueId } from "../../../render/webgl/TechniqueId";
import { ViewportQuadGeometry } from "../../../render/webgl/CachedGeometry";
import { Logger, LogLevel } from "@itwin/core-bentley";

function createPurpleQuadBuilder(): ProgramBuilder {
const builder = new ProgramBuilder(AttributeMap.findAttributeMap(undefined, false));
Expand Down Expand Up @@ -54,7 +55,9 @@ describe("Techniques", () => {
const useWebGL2 = webGLVersion === 2;
describe(`WebGL ${webGLVersion}`, () => {
before(async () => {
await IModelApp.startup({ renderSys: { useWebGL2 } });
await IModelApp.startup({ renderSys: { useWebGL2, errorOnMissingUniform: true } });
Logger.initializeToConsole();
Logger.setLevel("core-frontend.Render", LogLevel.Error);
});

after(async () => {
Expand Down Expand Up @@ -99,17 +102,22 @@ describe("Techniques", () => {
await IModelApp.startup({
renderSys: {
useWebGL2: false,
errorOnMissingUniform: true,
disabledExtensions: [disabledExtension],
},
});
Logger.initializeToConsole();
Logger.setLevel("core-frontend.Render", LogLevel.Error);
}

expect(System.instance.techniques.compileShaders()).to.be.true;

if (disabledExtension) {
// Reset render system to previous state
await IModelApp.shutdown();
await IModelApp.startup({ renderSys: { useWebGL2: false } });
await IModelApp.startup({ renderSys: { useWebGL2: false, errorOnMissingUniform: true } });
Logger.initializeToConsole();
Logger.setLevel("core-frontend.Render", LogLevel.Error);
}
}

Expand Down Expand Up @@ -181,7 +189,7 @@ describe("Techniques", () => {

expect(compiled).to.be.false;
expect(ex).not.to.be.undefined;
expect(ex!.toString().includes("uniform u_unused not found.")).to.be.true;
expect(ex!.toString().includes("uniform u_unused not found")).to.be.true;
});

describe("Number of varying vectors", () => {
Expand Down
2 changes: 2 additions & 0 deletions test-apps/display-test-app/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ You can use these environment variables to alter the default behavior of various
* If defined, do not allow visible or hidden edges to be displayed, and also do not create any UI related to them.
* IMJS_USE_WEBGL2
* Unless set to "0" or "false", the system will attempt to create a WebGL2 context before possibly falling back to WebGL1.
* IMJS_DISABLE_UNIFORM_ERRORS
* If defined, do not throw an error for missing shader uniforms, and call Logger instead.
* IMJS_MAX_TILES_TO_SKIP
* The number of levels of iModel tile trees to skip before loading graphics.
* IMJS_DEBUG_SHADERS
Expand Down
4 changes: 4 additions & 0 deletions test-apps/display-test-app/src/common/DtaConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface DtaConfiguration {
dpiAwareLOD?: boolean; // default OFF
disableEdges?: boolean; // default OFF
useWebGL2?: boolean; // default ON
errorOnMissingUniform?: boolean; // default true
debugShaders?: boolean; // default OFF
alwaysLoadEdges?: boolean; // default OFF
minimumSpatialTolerance?: number; // default undefined (no minimum)
Expand Down Expand Up @@ -108,6 +109,9 @@ export const getConfig = (): DtaConfiguration => {
if (undefined !== process.env.IMJS_DISABLE_BREP_CACHE)
configuration.disableBRepCache = true;

if (undefined !== process.env.IMJS_DISABLE_UNIFORM_ERRORS)
configuration.errorOnMissingUniform = false;

if (undefined !== process.env.IMJS_DEBUG_SHADERS)
configuration.debugShaders = true;

Expand Down
7 changes: 6 additions & 1 deletion test-apps/display-test-app/src/frontend/DisplayTestApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { ProcessDetector } from "@itwin/core-bentley";
import { Logger, LogLevel, ProcessDetector } from "@itwin/core-bentley";
import { CloudStorageContainerUrl, CloudStorageTileCache, RpcConfiguration, TileContentIdentifier } from "@itwin/core-common";
import { IModelApp, IModelConnection, RenderDiagnostics, RenderSystem, TileAdmin } from "@itwin/core-frontend";
import { WebGLExtensionName } from "@itwin/webgl-compatibility";
Expand Down Expand Up @@ -85,6 +85,7 @@ const dtaFrontendMain = async () => {
dpiAwareLOD: true === configuration.dpiAwareLOD,
useWebGL2: false !== configuration.useWebGL2,
planProjections: true,
errorOnMissingUniform: false !== configuration.errorOnMissingUniform,
debugShaders: true === configuration.debugShaders,
antialiasSamples: configuration.antialiasSamples,
};
Expand Down Expand Up @@ -156,6 +157,10 @@ const dtaFrontendMain = async () => {

await uiReady; // Now wait for the HTML UI to finish loading.
await initView(iModel);
Logger.initializeToConsole();
Logger.setLevelDefault(LogLevel.Warning);
Logger.setLevel("core-frontend.Render", LogLevel.Error);

if (configuration.startupMacro)
await IModelApp.tools.parseAndRun(`dta macro ${configuration.startupMacro}`);
} catch (reason) {
Expand Down

0 comments on commit adcd85d

Please sign in to comment.