Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix copyImageToCanvas bug and add ReadImageToCanvasOptions #7539

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d10e008
init commit
andremig-bentley Dec 19, 2024
cb438a7
create canvasDecorations view flag, fix copy image bug
andremig-bentley Jan 10, 2025
b9d78c3
cleanup
andremig-bentley Jan 10, 2025
18694a8
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 10, 2025
8e49ea3
rush change
andremig-bentley Jan 10, 2025
1066412
update rush change
andremig-bentley Jan 10, 2025
e39235e
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 21, 2025
e38bfa3
rm view flag changes
andremig-bentley Jan 21, 2025
570759b
cleanup, extract-api
andremig-bentley Jan 21, 2025
5b86b52
Add ReadImageToCanvasOptions, SaveImageWithCanvasDecorations Tool
andremig-bentley Jan 22, 2025
a19ad85
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 22, 2025
b3b93be
extract api
andremig-bentley Jan 22, 2025
0807157
rm SaveImageWithDecorations tool, update dta readme
andremig-bentley Jan 22, 2025
40fa886
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 22, 2025
1231e41
cleanup
andremig-bentley Jan 22, 2025
d683c4d
Merge branch 'andremig/decoration' of https://github.com/iTwin/itwinj…
andremig-bentley Jan 22, 2025
ae3eac8
NextVersion
andremig-bentley Jan 23, 2025
b600995
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 23, 2025
802c215
next version cleanup
andremig-bentley Jan 23, 2025
42b90c9
Merge branch 'andremig/decoration' of https://github.com/iTwin/itwinj…
andremig-bentley Jan 23, 2025
e593f01
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 24, 2025
6b71824
test progress, includeCanvasDecorations -> omitCanvasDecorations, con…
andremig-bentley Jan 24, 2025
c5255a6
Fix tests, finish readimagetocanvas implementations, update docs/read…
andremig-bentley Jan 28, 2025
31b16a2
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 28, 2025
f2aadcd
Update docs/changehistory/NextVersion.md
andremig-bentley Jan 28, 2025
c4925db
Update core/frontend/src/Viewport.ts
andremig-bentley Jan 28, 2025
39f693e
Update core/frontend/src/Viewport.ts
andremig-bentley Jan 28, 2025
411752c
add default false tests
andremig-bentley Jan 28, 2025
507c3ed
Merge branch 'master' into andremig/decoration
andremig-bentley Jan 28, 2025
8e28410
extract-api
andremig-bentley Jan 28, 2025
0b98c20
rm deprecated test
andremig-bentley Jan 28, 2025
7948a35
rm deprecated calls
andremig-bentley Jan 28, 2025
eae7cb4
rush change
andremig-bentley Jan 28, 2025
9b57306
Merge branch 'master' into andremig/decoration
andremig-bentley Feb 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions common/api/core-frontend.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -8235,7 +8235,7 @@ export class OffScreenTarget extends Target {
// (undocumented)
onResized(): void;
// (undocumented)
readImageToCanvas(): HTMLCanvasElement;
readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement;
// (undocumented)
setViewRect(rect: ViewRect, temporary: boolean): void;
// (undocumented)
Expand Down Expand Up @@ -8319,7 +8319,7 @@ export class OnScreenTarget extends Target {
// (undocumented)
pickOverlayDecoration(pt: XAndY): CanvasDecoration | undefined;
// (undocumented)
readImageToCanvas(): HTMLCanvasElement;
readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement;
// (undocumented)
setRenderToScreen(toScreen: boolean): HTMLCanvasElement | undefined;
// (undocumented)
Expand Down Expand Up @@ -9071,6 +9071,11 @@ export interface ReadImageBufferArgs {
upsideDown?: boolean;
}

// @public
export interface ReadImageToCanvasOptions {
includeCanvasDecorations?: boolean;
}

// @internal (undocumented)
export function readImdlContent(args: ImdlReaderCreateArgs & {
parseDocument?: (parseOpts: ImdlParserOptions) => Promise<ImdlModel.Document | ImdlParseError>;
Expand Down Expand Up @@ -10356,7 +10361,7 @@ export abstract class RenderTarget implements Disposable, RenderMemory.Consumer
// @internal (undocumented)
readImageBuffer(_args?: ReadImageBufferArgs): ImageBuffer | undefined;
// @internal (undocumented)
readImageToCanvas(): HTMLCanvasElement;
readImageToCanvas(_options?: ReadImageToCanvasOptions): HTMLCanvasElement;
// @internal (undocumented)
abstract readPixels(rect: ViewRect, selector: Pixel.Selector, receiver: Pixel.Receiver, excludeNonLocatable: boolean, excludedElements?: Iterable<Id64String>): void;
// @internal (undocumented)
Expand Down Expand Up @@ -11619,7 +11624,7 @@ export abstract class Target extends RenderTarget implements RenderTargetDebugCo
// (undocumented)
computeEdgeWeight(pass: RenderPass, baseWeight: number): number;
// (undocumented)
copyImageToCanvas(): HTMLCanvasElement;
copyImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement;
// (undocumented)
createPlanarClassifier(properties?: ActiveSpatialClassifier): PlanarClassifier;
// (undocumented)
Expand Down Expand Up @@ -14759,7 +14764,7 @@ export abstract class Viewport implements Disposable, TileUser {
// @deprecated
readImage(rect?: ViewRect, targetSize?: Point2d, flipVertically?: boolean): ImageBuffer | undefined;
readImageBuffer(args?: ReadImageBufferArgs): ImageBuffer | undefined;
readImageToCanvas(): HTMLCanvasElement;
readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement;
readPixels(rect: ViewRect, selector: Pixel.Selector, receiver: Pixel.Receiver, excludeNonLocatable?: boolean): void;
readPixels(args: ReadPixelsArgs): void;
// @internal
Expand Down
1 change: 1 addition & 0 deletions common/api/summary/core-frontend.exports.csv
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ public;function;readGltfGraphics
public;interface;ReadGltfGraphicsArgs
beta;function;readGltfTemplate
public;interface;ReadImageBufferArgs
public;interface;ReadImageToCanvasOptions
internal;function;readImdlContent
public;interface;ReadMeshArgs
internal;class;ReadonlyTileUserSet
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-frontend",
"comment": "Add ReadImageToCanvasOptions",
"type": "none"
}
],
"packageName": "@itwin/core-frontend"
}
12 changes: 10 additions & 2 deletions core/frontend/src/Viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ export interface ReadPixelsArgs {
excludedElements?: Iterable<Id64String>;
}

/** Arguments supplied to [[Viewport.readImageToCanvas]].
* @public
*/
export interface ReadImageToCanvasOptions {
/** If true, canvas decorations will be included in the saved image. */
andremig-bentley marked this conversation as resolved.
Show resolved Hide resolved
omitCanvasDecorations: boolean;
andremig-bentley marked this conversation as resolved.
Show resolved Hide resolved
}

/** A Viewport renders the contents of one or more [GeometricModel]($backend)s onto an `HTMLCanvasElement`.
*
* It holds a [[ViewState]] object that defines its viewing parameters; the ViewState in turn defines the [[DisplayStyleState]],
Expand Down Expand Up @@ -2708,8 +2716,8 @@ export abstract class Viewport implements Disposable, TileUser {
/** Reads the current image from this viewport into an HTMLCanvasElement with a Canvas2dRenderingContext such that additional 2d graphics can be drawn onto it.
* @see [[readImageBuffer]] to obtain the image as an array of RGBA pixels.
*/
public readImageToCanvas(): HTMLCanvasElement {
return this.target.readImageToCanvas();
public readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement {
return this.target.readImageToCanvas(options);
}

/** Used internally by `waitForSceneCompletion`.
Expand Down
4 changes: 2 additions & 2 deletions core/frontend/src/render/RenderTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Point2d, XAndY } from "@itwin/core-geometry";
import { IModelConnection } from "../IModelConnection";
import { HiliteSet } from "../SelectionSet";
import { SceneContext } from "../ViewContext";
import { ReadImageBufferArgs, Viewport } from "../Viewport";
import { ReadImageBufferArgs, ReadImageToCanvasOptions, Viewport } from "../Viewport";
import { ViewRect } from "../common/ViewRect";
import { CanvasDecoration } from "./CanvasDecoration";
import { Decorations } from "./Decorations";
Expand Down Expand Up @@ -176,7 +176,7 @@ export abstract class RenderTarget implements Disposable, RenderMemory.Consumer
/** @internal */
public readImageBuffer(_args?: ReadImageBufferArgs): ImageBuffer | undefined { return undefined; }
/** @internal */
public readImageToCanvas(): HTMLCanvasElement { return document.createElement("canvas"); }
public readImageToCanvas(_options?: ReadImageToCanvasOptions): HTMLCanvasElement { return document.createElement("canvas"); }
/** @internal */
public collectStatistics(_stats: RenderMemory.Statistics): void { }

Expand Down
82 changes: 64 additions & 18 deletions core/frontend/src/render/webgl/Target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { ViewRect } from "../../common/ViewRect";
import { canvasToImageBuffer, canvasToResizedCanvasWithBars, imageBufferToCanvas } from "../../common/ImageUtil";
import { HiliteSet, ModelSubCategoryHiliteMode } from "../../SelectionSet";
import { SceneContext } from "../../ViewContext";
import { ReadImageBufferArgs, Viewport } from "../../Viewport";
import { ReadImageBufferArgs, ReadImageToCanvasOptions, ScreenViewport, Viewport } from "../../Viewport";
import { IModelConnection } from "../../IModelConnection";
import { CanvasDecoration } from "../CanvasDecoration";
import { Decorations } from "../Decorations";
Expand Down Expand Up @@ -64,6 +64,7 @@ import { FrameStatsCollector } from "../FrameStats";
import { ActiveSpatialClassifier } from "../../SpatialClassifiersState";
import { AnimationNodeId } from "../../common/internal/render/AnimationNodeId";
import { _implementationProhibited } from "../../common/internal/Symbols";
import { IModelApp } from "../../IModelApp";

function swapImageByte(image: ImageBuffer, i0: number, i1: number) {
const tmp = image.data[i0];
Expand Down Expand Up @@ -157,6 +158,8 @@ export abstract class Target extends RenderTarget implements RenderTargetDebugCo
public displayNormalMaps = true;

public freezeRealityTiles = false;

protected _omitCanvasDecorations? = false;
public get shadowFrustum(): Frustum | undefined {
const map = this.solarShadowMap;
return map.isEnabled && map.isReady ? map.frustum : undefined;
Expand Down Expand Up @@ -1177,15 +1180,56 @@ export abstract class Target extends RenderTarget implements RenderTargetDebugCo
return image;
}

public copyImageToCanvas(): HTMLCanvasElement {
private getCanvasFromImageBuffer(): HTMLCanvasElement {
const image = this.readImageBuffer();
const canvas = undefined !== image ? imageBufferToCanvas(image, false) : undefined;
const retCanvas = undefined !== canvas ? canvas : document.createElement("canvas");
const pixelRatio = this.devicePixelRatio;
retCanvas.getContext("2d")!.scale(pixelRatio, pixelRatio);
return retCanvas;
}

private combineCanvasWithViewportOverlayCanvas(baseCanvas: HTMLCanvasElement, vp: ScreenViewport): HTMLCanvasElement {
const overlayCanvas = vp.canvas;
const ctx = baseCanvas.getContext("2d")!;
ctx.drawImage(overlayCanvas, 0, 0);
return baseCanvas;
}

private toggleOmitCanvasDecorations(hideDecorations: boolean, vp: ScreenViewport) {
this._omitCanvasDecorations = hideDecorations;
vp.invalidateScene();
}

public copyImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement {

/** There are four cases to consider:
* 1. Single viewport, with decorations: Must combine the webgl canvas with the 2d canvas, as canvas decorations are overlayed on top of the webgl canvas using the 2d canvas.
* 2. Single viewport, no decorations: Basic Copy image from image buffer. Decorations will be automaticaly excluded as they are overlayed on the separate 2d canvas.
* 3. Multiple viewports, with decorations: Basic copy image from image buffer. In this case, decorations will be included as they are not overlayed on a separate canvas
* when multiple viewports are present.
* 4. Multiple viewports, no decorations: Turn off decorations, copy image, then turn decorations back on.
*/

const vp = IModelApp.viewManager.selectedView;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, Target should not be doing anything with a Viewport, and there's no reason to believe this target is associated with the view manager's currently-selected view. If you need access to the Viewport's canvas, the Viewport should pass it in.

if (!vp)
return document.createElement("canvas");

// case 1
if (vp.rendersToScreen && !options?.omitCanvasDecorations) {
return this.combineCanvasWithViewportOverlayCanvas(this.getCanvasFromImageBuffer(), vp);
}

// case 4
if (!vp.rendersToScreen && options?.omitCanvasDecorations) {
this.toggleOmitCanvasDecorations(true, vp);
const retCanvas = this.getCanvasFromImageBuffer();
this.toggleOmitCanvasDecorations(false, vp);
return retCanvas;
}

// cases 2 and 3
return this.getCanvasFromImageBuffer();
}

public drawPlanarClassifiers() {
if (this._planarClassifiers) {
this._planarClassifiers.forEach((classifier) => {
Expand Down Expand Up @@ -1468,16 +1512,18 @@ export class OnScreenTarget extends Target {
this._2dCanvas.needsClear = false;
}

const canvasDecs = this.graphics.canvasDecorations;
if (canvasDecs) {
for (const overlay of canvasDecs) {
ctx.save();
if (overlay.position)
ctx.translate(overlay.position.x, overlay.position.y);

overlay.drawDecoration(ctx);
this._2dCanvas.needsClear = true;
ctx.restore();
if (!this._omitCanvasDecorations) {
const canvasDecs = this.graphics.canvasDecorations;
if (canvasDecs) {
for (const overlay of canvasDecs) {
ctx.save();
if (overlay.position)
ctx.translate(overlay.position.x, overlay.position.y);

overlay.drawDecoration(ctx);
this._2dCanvas.needsClear = true;
ctx.restore();
}
}
}
}
Expand Down Expand Up @@ -1512,8 +1558,8 @@ export class OnScreenTarget extends Target {
return toScreen ? this._webglCanvas.canvas : undefined;
}

public override readImageToCanvas(): HTMLCanvasElement {
return this._usingWebGLCanvas ? this.copyImageToCanvas() : this._2dCanvas.canvas;
public override readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement {
return this._usingWebGLCanvas ? this.copyImageToCanvas(options) : this._2dCanvas.canvas;
}
}

Expand Down Expand Up @@ -1556,8 +1602,8 @@ export class OffScreenTarget extends Target {
this.renderSystem.frameBufferStack.pop();
}

public override readImageToCanvas(): HTMLCanvasElement {
return this.copyImageToCanvas();
public override readImageToCanvas(options?: ReadImageToCanvasOptions): HTMLCanvasElement {
return this.copyImageToCanvas(options);
}
}

Expand Down
103 changes: 102 additions & 1 deletion core/frontend/src/test/Viewport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
AnalysisStyle, ColorDef, EmptyLocalization, Feature, ImageBuffer, ImageBufferFormat, ImageMapLayerSettings,
} from "@itwin/core-common";
import { ViewRect } from "../common/ViewRect";
import { OffScreenViewport, ScreenViewport, Viewport } from "../Viewport";
import { OffScreenViewport, ReadImageToCanvasOptions, ScreenViewport, Viewport } from "../Viewport";
import { DisplayStyle3dState } from "../DisplayStyleState";
import { SpatialViewState } from "../SpatialViewState";
import { IModelApp } from "../IModelApp";
Expand All @@ -21,6 +21,7 @@ import { Pixel } from "../render/Pixel";
import { GraphicType } from "../common/render/GraphicType";
import { RenderGraphic } from "../render/RenderGraphic";
import { Decorator } from "../ViewManager";
import { CanvasDecoration } from "../core-frontend";

describe("Viewport", () => {
beforeAll(async () => IModelApp.startup({ localization: new EmptyLocalization() }));
Expand Down Expand Up @@ -716,4 +717,104 @@ describe("Viewport", () => {
});
});
});

describe("Read Image To Canvas", () => {

class PixelCanvasDecoration implements CanvasDecoration {
public drawDecoration(ctx: CanvasRenderingContext2D) {
ctx.fillStyle = "red";
ctx.fillRect(0,0,1,1);
}
}

class PixelCanvasDecorator implements Decorator {
public decorate(context: DecorateContext) {

const builder = context.createGraphicBuilder(GraphicType.WorldDecoration, undefined);
builder.addLineString2d([new Point2d(0,0), new Point2d(1,1)],0);
context.addDecorationFromBuilder(builder);
context.addCanvasDecoration(new PixelCanvasDecoration());
}
}

function createViewport(): ScreenViewport {
const state = SpatialViewState.createBlank(createBlankConnection(), { x: 0, y: 0, z: 0 }, { x: 1, y: 1, z: 1 })
const parentDiv = document.createElement("div");
parentDiv.setAttribute("height", "100px");
parentDiv.setAttribute("width", "100px");
parentDiv.style.height = parentDiv.style.width = "100px";
document.body.appendChild(parentDiv);
return ScreenViewport.create(parentDiv, state);
}

const activeDecorators: Decorator[] = [];
function addDecorator(dec: Decorator) {
IModelApp.viewManager.dropDecorator(dec);
IModelApp.viewManager.addDecorator(dec);
activeDecorators.push(dec);
}

function getPixelRgb(pixel: Uint8ClampedArray): [number, number, number] {
return [pixel[3], pixel[2], pixel[1]];
}

afterEach(() => {
for (const dec of activeDecorators) {
IModelApp.viewManager.dropDecorator(dec);
}

activeDecorators.length = 0;
});

it("should not include canvas decorations if omitCanvasDecorations is true", () => {
const vp = createViewport();
andremig-bentley marked this conversation as resolved.
Show resolved Hide resolved
IModelApp.viewManager.addViewport(vp);

addDecorator(new PixelCanvasDecorator());
vp.renderFrame();

const readImageOptions: ReadImageToCanvasOptions = {
omitCanvasDecorations: true,
};

const canvas = vp.readImageToCanvas(readImageOptions);
const ctx = canvas.getContext("2d");
const pixel = ctx!.getImageData(0, 0, 1, 1).data;
const rgb = getPixelRgb(pixel);
expect(rgb).toEqual([0,0,0]);



IModelApp.viewManager.dropViewport(vp);
});

it("should include canvas decorations if omitCanvasDecorations is false", () => {
const vp = createViewport();
IModelApp.viewManager.addViewport(vp);

addDecorator(new PixelCanvasDecorator());
vp.renderFrame();

const readImageOptions: ReadImageToCanvasOptions = {
omitCanvasDecorations: false,
};

const canvas = vp.readImageToCanvas(readImageOptions);
const ctx = canvas.getContext("2d");
const pixel = ctx!.getImageData(0, 0, 1, 1).data;
const rgb = getPixelRgb(pixel);
expect(rgb).toEqual([255,0,0]);

IModelApp.viewManager.dropViewport(vp);
});

/**
* Other changes to the above tests which produced the same bug-free behavior (not what we want):
* Use openBlankViewport to get a blank viewport instead of creating a screen viewport directly
* Set vp.rendersToScreen to true before calling vp.renderFrame (for both vp types)
* Use vp.target.SetRenderToScreen to do the same thing
* Making sure all Viewports are removed before adding a new one at the beginning of the test
* Fully shutting down IModel App and then restarting it at the beginning of the test
*/
});
});
8 changes: 8 additions & 0 deletions docs/changehistory/NextVersion.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Table of contents:
- [Change to pullMerge](#change-to-pullmerge)
- [No pending/local changes](#no-pendinglocal-changes)
- [With pending/local changes](#with-pendinglocal-changes)
- [Graphics](#graphics)
- [Read Image To Canvas](#read-image-to-canvas)

## Selection set

Expand Down Expand Up @@ -286,3 +288,9 @@ This method offers several advantages:
4. In the future, this method will be essential for lock-less editing as it enables applications to merge changes with domain intelligence.

For more information read [Pull merge & conflict resolution](../learning/backend/PullMerge.md)

## Graphics

### Read Image To Canvas

Previously, when using [Viewport.readImageToCanvas]($core-frontend) with a single open viewport, canvas decorations were not included in the saved image. Sometimes this behavior was useful, so the [ReadImageToCanvasOptions]($core-frontend) interface was created to allow the option to choose whether or not canvas decorations are included in the saved image. Now, if [ReadImageToCanvasOptions.includeCanvasDecorations]($core-frontend) is true, canvas decorations will be included in the saved image. If undefined or false, previous behavior will persist and canvas decorations will not be included. All existing calls to [Viewport.readImageToCanvas]($core-frontend) will be unaffected by this change as the inclusion of [ReadImageToCanvasOptions]($core-frontend) is optional, and when they are undefined, previous behavior will persist.
Loading
Loading