Add Draw API #7628

Linchenn · 2023-04-25T20:07:25Z

This PR add Draw op to Core and implemented it in CPU backend.

We could get the following effects with a couple lines of codes:

// Setup
tf.setBackend('cpu');
const imgElem = document.getElementById('salad');
const canvasElem = document.getElementById("display");
const image = tf.browser.fromPixels(imgElem);

// TFJS processing
const filter = tf.tensor4d(fval, [3,3,3,3], 'float32');
const normalizedImage = tf.div(image, 255);
const convRes = tf.conv2d(normalizedImage, filter, 1, 'same');
const res =  tf.clipByValue(convRes, 0, 1);

// Draw
const drawOptions = {imageOptions: {alpha: 0.5}};
tf.browser.draw(res, canvasElem, drawOptions);

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

Linchenn · 2023-04-27T01:57:36Z

The tests for this PR is under CPU-backend, instead of Core, because different backend would use different context and has different implementation to validate the image data.

Linchenn · 2023-04-27T01:59:27Z

@axinging Could you help review this PR? Hope the API design make sense for WebGPU backend.

qjia7 · 2023-04-27T02:13:58Z

tfjs-backend-cpu/src/kernels/Draw.ts

+  }
+  const ctx = canvas.getContext(contextType,
+    canvasOptions?.contextAttributes || {}) as CanvasRenderingContext2D ;
+


Check whether ctx is null in case getContext is called before with a different contextType?

qjia7 · 2023-04-27T02:24:33Z

tfjs-backend-cpu/src/kernels/Draw.ts

+      const value = data[i * depth + d];
+
+      if (image.dtype === 'float32') {
+        if (value < 0 || value > 1) {


Should we throw an error or just clamp it to the supported range? For webgl/webgpu backend, it's not possible to do this check and throw errors.

For WebGL and WebGPU, I don't think we need to check values and we could just clamp them. For CPU, it's very convenient to make such checks and throwing an error would be helpful for users, because it's common that users pass values out of 255 and get a pure white canvas with no hit message. So, I think this throw-error feature is good to have.

qjia7 · 2023-04-27T02:38:19Z

tfjs-core/src/ops/browser.ts

-    throw new Error(
-        `toPixels only supports rank 2 or 3 tensors, got rank ${$img.rank}.`);
-  }
+  validateImgTensor($img);


Maybe add a warning in this API to tell users that this API will be deprecated and suggest them switch to the more powerful draw API.

Done. Thanks!

qjia7 · 2023-04-27T02:45:47Z

tfjs-core/src/ops/browser.ts

+ */
+export function draw(
+    image: Tensor2D|Tensor3D|TensorLike, canvas: HTMLCanvasElement,
+    contextOptions?: ContextOptions, drawOptions?: DrawOptions): void {


Is it simpler to merge contextOptions and drawOptions into CanvasOptions/CanvasConfiguration? For example, in webgpu, we have GPUCanvasConfiguration

export interface CanvasOptions { /** * Optional. If it is not set, it would be variable based on the current * backend. */ contextType?: string; /** * Optional. */ contextAttributes?: WebGLContextAttributes; /** * Optional. A number in range [0-1]. If the image is a 2D tensor or a 3D * tensor with 1 or 3 channels, the alpha channels would set as its value; * otherwise, it would not make effects. */ alpha?: number; }

GPUCanvasConfiguration has alphaMode, but it is different from the alpha here.

I mean either contextOptions or GPUCanvasConfiguration are describing the setting of canvas or context. However, drawOptions is to customize the kernel computations. Separating them might be more clearer for users?

Also, do you think the name and the definition of contextOptions makes sense for WebGPU?

I think it's ok to custom alpha. I don't have good candidate name for drawOptions. Leave it open if other reviewers have better suggestion or just keep the current name.

Sorry, I mean contextOptions (updated my comment), does it makes senses for WebGPU? Should I add more optional fields, like GPUCanvasConfiguration? I intend to wrap all canvas and context related settings into it.

I see. It will be good to change contextAttributes?: WebGLContextAttributes; -> contextAttributes?: WebGLContextAttributes|GPUCanvasConfiguration;. GPUCanvasConfiguration makes it possible that draw the tensor to a different device's canvas since it has the device member.

One issue is that webgpu allows "opaque" alphaMode. It may conflict with alpha in drawOption. We need to define the priority. Once alphaMode is opaque. alpha will take no effect or alpha will override alphaMode?

I agree that merging two options into one can cause some confusion in some overlapping but different purpose params like alpha. But I feel a little bit concerned when I see your example usage code:

tf.browser.draw(res, canvasElem, undefined, {alpha: 0.5});

From the user's perspective it is hard to tell whether the first one or second one is drawOptions. We may hurt the readability and usability of this API. Just my personal concerns, throw it out for more discussion. Feel free to disagree.

Also, if we choose to release it as two separate options params, consider to make it:

function draw( image: Tensor2D|Tensor3D|TensorLike, canvas: HTMLCanvasElement, drawOptions?: DrawOptions, contextOptions?: ContextOptions)

if you think customizing drawOptions is a normal user feature but specifying contextOptions is more like an feature for advanced users, swapping the order may be more easy to use since most of the users can just do tf.browser.draw(res, canvas, { alpha: 1, ...});

Since we would add GPUCanvasConfiguration to CanvasOptions, I renamed ContextOptions to CanvasOptions.

For usage of the API with two objects, if users follow the coding style, it would be:

const canvasOptions = {}; const drawOptions = {alpha: 0.5} tf.browser.draw(res, canvasElem, canvasOptions, drawOptions);

I am open to suggestions for whether we want to merge the two options as Options: {CanvasOptions, DrawOptions}.

As collected feedbacks from you and WebML channel, get most votes for unified option object for this. Updated.

qjia7 · 2023-04-27T02:55:33Z

The tests for this PR is under CPU-backend, instead of Core, because different backend would use different context and has different implementation to validate the image data.

Maybe we can easily let each backend support both backend preferred canvas context + 2d canvas. Then we can always use 2d canvas to verify the correctness in core.

Linchenn · 2023-04-27T06:23:17Z

The tests for this PR is under CPU-backend, instead of Core, because different backend would use different context and has different implementation to validate the image data.

Maybe we can easily let each backend support both backend preferred canvas context + 2d canvas. Then we can always use 2d canvas to verify the correctness in core.

Nice catch, done. Thanks!

qjia7

LGTM, thanks!

qjia7 · 2023-04-27T06:53:43Z

tfjs-core/src/ops/browser.ts

+ */
+export function draw(
+    image: Tensor2D|Tensor3D|TensorLike, canvas: HTMLCanvasElement,
+    contextOptions?: ContextOptions, drawOptions?: DrawOptions): void {


I think it's ok to custom alpha. I don't have good candidate name for drawOptions. Leave it open if other reviewers have better suggestion or just keep the current name.

chunnienc · 2023-04-27T18:38:28Z

tfjs-core/src/ops/browser.ts

+ * [0-255].
+ *
+ * @param image A rank-2 tensor with shape `[height, width]`, or a rank-3 tensor
+ * of shape `[height, width, numChannels]`. If rank-2, draws grayscale. If


"draws grayscale" -> "draws in grayscale"
I feel like the comment can be written to be more clear. @mattsoulanille please help us with grammar.

tfjs-core/src/ops/draw_test.ts

chunnienc

LGTM except for:

The doc comment can be improved. @mattsoulanille for native speaker review.
The API with two options looks okay to me, but we can have more discussion on that if you want online and/or offline.

mattsoulanille · 2023-04-27T20:22:44Z

tfjs-core/src/ops/browser.ts

+    console.warn(
+        'tf.browser.toPixels is not efficient to draw tensor on canvas. ' +
+        'Please try tf.browser.draw instead.');


Can we print this warning only once? Otherwise, we may spam the console if the user is running toPixels in a loop / requestAnimationFrame.

Nice catch. Added a variable in the browser.ts file for the boolean, since the file already has a variable hasToPixelsWarned, which share the coding style.

mattsoulanille · 2023-04-27T20:34:58Z

tfjs-core/src/ops/draw_test.ts

+  }
+}
+
+describeWithFlags('Draw on 2d context', BROWSER_ENVS, () => {


Nit: If 'Draw on 2d context' should never be tested in WebGL, then we could use a different set of backends than BROWSER_ENVS instead of filtering it out in setup_test.ts. Alternatively, if we intend to implement drawing on a 2d canvas from WebGL in the future, then the current filtering approach looks good.

On the other hand, this might be more complicated than simply filtering the test case out, so feel free to leave it as-is.

As Jiajia suggested, we would implement and test 2d context for all backends soon.

tfjs-core/src/ops/draw_test.ts

tfjs-core/src/ops/browser.ts

mattsoulanille · 2023-05-01T18:36:40Z

tfjs-core/src/ops/browser.ts

+ * @param canvasOptions A object to configure the canvas to draw to.
+ * @param drawOptions A object of options to customize drawing.


Should we describe the settings that people can change on these config and draw options here, or is this documented elsewhere?

This is at the definition of drawOptions and canvasOptions and tfjs-website would add them to the API. Similar as Options of this API.

Co-authored-by: Matthew Soulanille <[email protected]>

Jonathhhan · 2023-06-07T17:23:54Z

Thanks for the nice lib. I also get the warning: tf.browser.toPixels is not efficient to draw tensor on canvas. Please try tf.browser.draw instead. It only works with tf.setBackend('cpu');, but then it is (in total) much slower than with tf.setBackend('webgl'). Are there any plans to implement tf.browser.draw for the webgl backend?

Linchenn and others added 6 commits April 25, 2023 13:03

add draw

14870cd

add tests

fce7b9d

add comments

0799d0b

Merge branch 'master' into draw

a715556

lint

3b6c0f9

Merge branch 'draw' of https://github.com/Linchenn/tfjs into draw

0bb099a

Linchenn requested review from mattsoulanille, qjia7 and chunnienc April 27, 2023 01:58

qjia7 reviewed Apr 27, 2023

View reviewed changes

upd

d08b41b

Linchenn requested a review from qjia7 April 27, 2023 06:23

qjia7 approved these changes Apr 27, 2023

View reviewed changes

chunnienc requested changes Apr 27, 2023

View reviewed changes

typo

89b93e1

Linchenn requested a review from chunnienc April 27, 2023 20:20

chunnienc approved these changes Apr 28, 2023

View reviewed changes

Linchenn added 2 commits April 28, 2023 18:19

cmt

10506a6

rename contextOption to canvasOption

6851210

Linchenn requested review from pyu10055 and gyagp May 1, 2023 17:47

Merge branch 'master' into draw

cc3d46b

mattsoulanille reviewed May 1, 2023

View reviewed changes

Linchenn and others added 3 commits May 1, 2023 12:37

Update tfjs-core/src/ops/browser.ts

3b7513b

Co-authored-by: Matthew Soulanille <[email protected]>

Update tfjs-core/src/ops/draw_test.ts

d72a3da

Co-authored-by: Matthew Soulanille <[email protected]>

warn once

5e3cdee

lint

46b09a4

mattsoulanille approved these changes May 1, 2023

View reviewed changes

Linchenn and others added 10 commits May 5, 2023 18:03

unify options

05b25e9

Merge branch 'master' into draw

871f2c9

typo

b1d1710

tune

a0e5109

Merge branch 'draw' of https://github.com/Linchenn/tfjs into draw

5ddce0c

tune

3b3a37f

Merge branch 'master' into draw

53472ae

tune

42fa812

Merge branch 'draw' of https://github.com/Linchenn/tfjs into draw

b82b98f

typo

812f3f7

Linchenn merged commit db099a4 into tensorflow:master May 6, 2023

Linchenn deleted the draw branch May 6, 2023 23:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Draw API #7628

Add Draw API #7628

Linchenn commented Apr 25, 2023 •

edited

Loading

Linchenn commented Apr 27, 2023

Linchenn commented Apr 27, 2023

qjia7 Apr 27, 2023

Linchenn Apr 27, 2023

qjia7 Apr 27, 2023

Linchenn Apr 27, 2023

qjia7 Apr 27, 2023

Linchenn Apr 27, 2023

qjia7 Apr 27, 2023

Linchenn Apr 27, 2023 •

edited

Loading

qjia7 Apr 27, 2023

Linchenn Apr 27, 2023

qjia7 Apr 27, 2023

chunnienc Apr 28, 2023 •

edited

Loading

Linchenn May 1, 2023 •

edited

Loading

Linchenn May 6, 2023

qjia7 commented Apr 27, 2023

Linchenn commented Apr 27, 2023

qjia7 left a comment

qjia7 Apr 27, 2023

chunnienc Apr 27, 2023

Linchenn Apr 27, 2023

chunnienc left a comment

mattsoulanille Apr 27, 2023

Linchenn May 1, 2023

mattsoulanille Apr 27, 2023

Linchenn May 1, 2023

mattsoulanille May 1, 2023

Linchenn May 1, 2023

Jonathhhan commented Jun 7, 2023 •

edited

Loading

		* @param canvasOptions A object to configure the canvas to draw to.
		* @param drawOptions A object of options to customize drawing.

Add Draw API #7628

Add Draw API #7628

Conversation

Linchenn commented Apr 25, 2023 • edited Loading

Linchenn commented Apr 27, 2023

Linchenn commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Linchenn Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunnienc Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

Linchenn May 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qjia7 commented Apr 27, 2023

Linchenn commented Apr 27, 2023

qjia7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunnienc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jonathhhan commented Jun 7, 2023 • edited Loading

Linchenn commented Apr 25, 2023 •

edited

Loading

Linchenn Apr 27, 2023 •

edited

Loading

chunnienc Apr 28, 2023 •

edited

Loading

Linchenn May 1, 2023 •

edited

Loading

Jonathhhan commented Jun 7, 2023 •

edited

Loading