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

Add APIs for IO card rendering #2739

Merged
merged 10 commits into from
Oct 19, 2023
4 changes: 2 additions & 2 deletions proto/anki/image_occlusion.proto
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ message GetImageOcclusionNoteResponse {
repeated ImageOcclusionProperty properties = 2;
}

message ImageClozeNote {
message ImageOcclusionNote {
bytes image_data = 1;
repeated ImageOcclusion occlusions = 2;
string header = 3;
Expand All @@ -73,7 +73,7 @@ message GetImageOcclusionNoteResponse {
}

oneof value {
ImageClozeNote note = 1;
ImageOcclusionNote note = 1;
string error = 2;
}
}
Expand Down
9 changes: 6 additions & 3 deletions rslib/src/image_occlusion/imagedata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::path::PathBuf;

use anki_io::metadata;
use anki_io::read_file;
use anki_proto::image_occlusion::get_image_occlusion_note_response::ImageClozeNote;
use anki_proto::image_occlusion::get_image_occlusion_note_response::ImageOcclusionNote;
use anki_proto::image_occlusion::get_image_occlusion_note_response::Value;
use anki_proto::image_occlusion::AddImageOcclusionNoteRequest;
use anki_proto::image_occlusion::GetImageForOcclusionResponse;
Expand Down Expand Up @@ -82,9 +82,12 @@ impl Collection {
Ok(GetImageOcclusionNoteResponse { value: Some(value) })
}

pub fn get_image_occlusion_note_inner(&mut self, note_id: NoteId) -> Result<ImageClozeNote> {
pub fn get_image_occlusion_note_inner(
&mut self,
note_id: NoteId,
) -> Result<ImageOcclusionNote> {
let note = self.storage.get_note(note_id)?.or_not_found(note_id)?;
let mut cloze_note = ImageClozeNote::default();
let mut cloze_note = ImageOcclusionNote::default();

let fields = note.fields();

Expand Down
2 changes: 1 addition & 1 deletion rslib/src/image_occlusion/notetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub(crate) fn image_occlusion_notetype(tr: &I18n) -> Notetype {
</div>
<script>
try {{
anki.setupImageCloze();
anki.imageOcclusion.setup();
}} catch (exc) {{
document.getElementById("err").innerHTML = `{err_loading}<br><br>${{exc}}`;
}}
Expand Down
135 changes: 94 additions & 41 deletions ts/image-occlusion/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,57 @@
import * as tr from "@tslib/ftl";

import { optimumPixelSizeForCanvas } from "./canvas-scale";
import type { Shape } from "./shapes/base";
import { Ellipse } from "./shapes/ellipse";
import { extractShapesFromRenderedClozes } from "./shapes/from-cloze";
import { Polygon } from "./shapes/polygon";
import { Rectangle } from "./shapes/rectangle";
import { Text } from "./shapes/text";
import { Shape } from "./shapes";
import { Ellipse, extractShapesFromRenderedClozes, Polygon, Rectangle, Text } from "./shapes";
import { TEXT_BACKGROUND_COLOR, TEXT_FONT_FAMILY, TEXT_PADDING } from "./tools/lib";
import type { Size } from "./types";

export function setupImageCloze(): void {
export type DrawShapesData = {
activeShapes: Shape[];
inactiveShapes: Shape[];
properties: ShapeProperties;
};

export type DrawShapesFilter = (
data: DrawShapesData,
context: CanvasRenderingContext2D,
) => DrawShapesData | void;

export type DrawShapesCallback = (
data: DrawShapesData,
context: CanvasRenderingContext2D,
) => void;

export const imageOcclusionAPI = {
setup: setupImageOcclusion,
drawShape,
Ellipse,
Polygon,
Rectangle,
Shape,
Text,
};

interface SetupImageOcclusionOptions {
onWillDrawShapes?: DrawShapesFilter;
onDidDrawShapes?: DrawShapesCallback;
}

function setupImageOcclusion(setupOptions?: SetupImageOcclusionOptions): void {
window.addEventListener("load", () => {
window.addEventListener("resize", setupImageCloze);
window.addEventListener("resize", () => setupImageOcclusion(setupOptions));
});
window.requestAnimationFrame(setupImageClozeInner);
window.requestAnimationFrame(() => setupImageOcclusionInner(setupOptions));
}

function setupImageClozeInner(): void {
function setupImageOcclusionInner(setupOptions?: SetupImageOcclusionOptions): void {
const canvas = document.querySelector(
"#image-occlusion-canvas",
) as HTMLCanvasElement | null;
if (canvas == null) {
return;
}

const ctx: CanvasRenderingContext2D = canvas.getContext("2d")!;
const container = document.getElementById(
"image-occlusion-container",
) as HTMLDivElement;
Expand Down Expand Up @@ -56,47 +82,74 @@ function setupImageClozeInner(): void {
button.addEventListener("click", toggleMasks);
}

drawShapes(canvas, ctx);
drawShapes(canvas, setupOptions?.onWillDrawShapes, setupOptions?.onDidDrawShapes);
}

function drawShapes(
canvas: HTMLCanvasElement,
ctx: CanvasRenderingContext2D,
onWillDrawShapes?: DrawShapesFilter,
onDidDrawShapes?: DrawShapesCallback,
): void {
const properties = getShapeProperties();
const context: CanvasRenderingContext2D = canvas.getContext("2d")!;
const size = canvas;
for (const shape of extractShapesFromRenderedClozes(".cloze")) {
drawShape(ctx, size, shape, properties, true);

let activeShapes = extractShapesFromRenderedClozes(".cloze");
let inactiveShapes = extractShapesFromRenderedClozes(".cloze-inactive");
let properties = getShapeProperties();

const processed = onWillDrawShapes?.({ activeShapes, inactiveShapes, properties }, context);
if (processed) {
activeShapes = processed.activeShapes;
inactiveShapes = processed.inactiveShapes;
properties = processed.properties;
}
for (const shape of extractShapesFromRenderedClozes(".cloze-inactive")) {
if (shape.occludeInactive) {
drawShape(ctx, size, shape, properties, false);
}

for (const shape of activeShapes) {
drawShape({
context,
size,
shape,
fill: properties.activeShapeColor,
stroke: properties.activeBorder.color,
strokeWidth: properties.activeBorder.width,
});
}
for (const shape of inactiveShapes.filter((s) => s.occludeInactive)) {
drawShape({
context,
size,
shape,
fill: properties.inActiveShapeColor,
stroke: properties.inActiveBorder.color,
strokeWidth: properties.inActiveBorder.width,
});
}

onDidDrawShapes?.({ activeShapes, inactiveShapes, properties }, context);
}

function drawShape(
ctx: CanvasRenderingContext2D,
size: Size,
shape: Shape,
properties: ShapeProperties,
active: boolean,
): void {
shape.makeAbsolute(size);
interface DrawShapeParameters {
context: CanvasRenderingContext2D;
size: Size;
shape: Shape;
fill: string;
stroke: string;
strokeWidth: number;
}

const { color, border } = active
? {
color: properties.activeShapeColor,
border: properties.activeBorder,
}
: {
color: properties.inActiveShapeColor,
border: properties.inActiveBorder,
};
function drawShape({
context: ctx,
size,
shape,
fill,
stroke,
strokeWidth,
}: DrawShapeParameters): void {
shape = shape.toAbsolute(size);

ctx.fillStyle = color;
ctx.strokeStyle = border.color;
ctx.lineWidth = border.width;
ctx.fillStyle = fill;
ctx.strokeStyle = stroke;
ctx.lineWidth = strokeWidth;
if (shape instanceof Rectangle) {
ctx.fillRect(shape.left, shape.top, shape.width, shape.height);
ctx.strokeRect(shape.left, shape.top, shape.width, shape.height);
Expand Down Expand Up @@ -175,7 +228,7 @@ function topLeftOfPoints(points: { x: number; y: number }[]): {
return { x: left, y: top };
}

type ShapeProperties = {
export type ShapeProperties = {
activeShapeColor: string;
inActiveShapeColor: string;
activeBorder: { width: number; color: string };
Expand Down
40 changes: 31 additions & 9 deletions ts/image-occlusion/shapes/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ export class Shape {
* question side.
*/
occludeInactive = false;
/* Cloze ordinal */
ordinal = 0;

constructor(
{ left = 0, top = 0, fill = SHAPE_MASK_COLOR, occludeInactive = false }: ConstructorParams<Shape> = {},
{ left = 0, top = 0, fill = SHAPE_MASK_COLOR, occludeInactive = false, ordinal = 0 }: ConstructorParams<Shape> =
{},
) {
this.left = left;
this.top = top;
this.fill = fill;
this.occludeInactive = occludeInactive;
this.ordinal = ordinal;
}

/** Format numbers and remove default values, for easier serialization to
Expand All @@ -46,18 +50,36 @@ export class Shape {
}

toFabric(size: Size): fabric.ForCloze {
this.makeAbsolute(size);
return new fabric.Object(this);
const absolute = this.toAbsolute(size);
return new fabric.Object(absolute);
}

makeNormal(size: Size): void {
this.left = xToNormalized(size, this.left);
this.top = yToNormalized(size, this.top);
normalPosition(size: Size) {
return {
left: xToNormalized(size, this.left),
top: yToNormalized(size, this.top),
};
}

toNormal(size: Size): Shape {
return new Shape({
...this,
...this.normalPosition(size),
});
}

absolutePosition(size: Size) {
return {
left: xFromNormalized(size, this.left),
top: yFromNormalized(size, this.top),
};
}

makeAbsolute(size: Size): void {
this.left = xFromNormalized(size, this.left);
this.top = yFromNormalized(size, this.top);
toAbsolute(size: Size): Shape {
return new Shape({
...this,
...this.absolutePosition(size),
});
}
}

Expand Down
26 changes: 16 additions & 10 deletions ts/image-occlusion/shapes/ellipse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,26 @@ export class Ellipse extends Shape {
}

toFabric(size: Size): fabric.Ellipse {
this.makeAbsolute(size);
return new fabric.Ellipse(this);
const absolute = this.toAbsolute(size);
return new fabric.Ellipse(absolute);
}

makeNormal(size: Size): void {
super.makeNormal(size);
this.rx = xToNormalized(size, this.rx);
this.ry = yToNormalized(size, this.ry);
toNormal(size: Size): Ellipse {
return new Ellipse({
...this,
...super.normalPosition(size),
rx: xToNormalized(size, this.rx),
ry: yToNormalized(size, this.ry),
});
}

makeAbsolute(size: Size): void {
super.makeAbsolute(size);
this.rx = xFromNormalized(size, this.rx);
this.ry = yFromNormalized(size, this.ry);
toAbsolute(size: Size): Ellipse {
return new Ellipse({
...this,
...super.absolutePosition(size),
rx: xFromNormalized(size, this.rx),
ry: yFromNormalized(size, this.ry),
});
}
}

Expand Down
1 change: 1 addition & 0 deletions ts/image-occlusion/shapes/from-cloze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ function extractShapeFromRenderedCloze(cloze: HTMLDivElement): Shape | null {
}
const props = {
occludeInactive: cloze.dataset.occludeinactive === "1",
ordinal: parseInt(cloze.dataset.ordinal!),
left: cloze.dataset.left,
top: cloze.dataset.top,
width: cloze.dataset.width,
Expand Down
9 changes: 9 additions & 0 deletions ts/image-occlusion/shapes/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html

export { Shape } from "./base";
export { Ellipse } from "./ellipse";
export { extractShapesFromRenderedClozes } from "./from-cloze";
export { Polygon } from "./polygon";
export { Rectangle } from "./rectangle";
export { Text } from "./text";
Copy link
Member

Choose a reason for hiding this comment

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

Some people consider this an anti-pattern, and I've started to feel the same way. I know we have other examples in the codebase already that do this, but I'd suggest going forward, we limit this to public API exports, and avoid making these for our own use.

https://marvinh.dev/blog/speeding-up-javascript-ecosystem-part-7/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can definitely see that. I would also agree that the convenience factor in internal use really isn't that high.

However, given that third-party code consuming these APIs will likely also want to have access to their corresponding types, I'm wondering how we should best handle this here.

E.g., with the current state I can just import the various Shape types from this barrel file without having to worry about the internal structure of the shapes package (annotating shape types with e.g. typeof globalThis.anki.imageOcclusion.Shape doesn't work without some major TS acrobatics which are likely a bad practice of their own).

Perhaps just bundling type exports in barrel files like these could be an option? Or splitting out type exports in a different way like d.ts files?

FWIW, I don't feel particularly strong about this from a stylistic standpoint, just want to make sure that we go with as defensive a solution as possible, so that using anki types is not too brittle long-term.

Copy link
Member

Choose a reason for hiding this comment

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

If you're consuming the types then it's arguably public API, and I remove my objection.

I'd like to return to the question of the best way to expose these to add-on authors in the future - some previous discussion about it can be found at #1764.

Loading