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

chore(TS) remove ts-nocheck from image class #9036

Merged
merged 14 commits into from
Jul 10, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- chore(TS): Image class type checks, BREAKING change to FromURL static method [#9036](https://github.com/fabricjs/fabric.js/pull/9036)
- ci(): properly checkout head for stats [#9080](https://github.com/fabricjs/fabric.js/pull/9080)
- fix(Text): `_getFontDeclaration` wasn't considering fontFamily from the style object [#9082](https://github.com/fabricjs/fabric.js/pull/9082)
- chore(TS): Fix ITextBehaviour enterEditing type [#9075](https://github.com/fabricjs/fabric.js/pull/9075)
Expand Down
2 changes: 1 addition & 1 deletion src/canvas/StaticCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1414,7 +1414,7 @@ export class StaticCanvas<
this.renderOnAddRemove = false;

return Promise.all([
enlivenObjects(objects, {
enlivenObjects<FabricObject>(objects, {
reviver,
signal,
}),
Expand Down
3 changes: 2 additions & 1 deletion src/filters/WebGLFilterBackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
TTextureCache,
TPipelineResources,
} from './typedefs';
import type { BaseFilter } from './BaseFilter';

export class WebGLFilterBackend {
declare tileSize: number;
Expand Down Expand Up @@ -150,7 +151,7 @@ export class WebGLFilterBackend {
* omitted, caching will be skipped.
*/
applyFilters(
filters: any[],
filters: BaseFilter[],
source: TexImageSource,
width: number,
height: number,
Expand Down
3 changes: 2 additions & 1 deletion src/mixins/eraser_brush.mixin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ts-nocheck
import { Point } from '../Point';
import { FabricObject } from '../shapes/Object/FabricObject';
import { uid } from '../util/internals/uid';

(function (global) {
Expand Down Expand Up @@ -293,7 +294,7 @@ import { uid } from '../util/internals/uid';
options = fabric.util.object.clone(object, true);
delete options.objects;
return Promise.all([
fabric.util.enlivenObjects(objects),
fabric.util.enlivenObjects<FabricObject>(objects),
fabric.util.enlivenObjectEnlivables(options),
]).then(function (enlivedProps) {
return new fabric.Eraser(
Expand Down
2 changes: 1 addition & 1 deletion src/shapes/Group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ export class Group extends createCollectionMixin(
...options
}: T) {
return Promise.all([
enlivenObjects(objects),
enlivenObjects<FabricObject>(objects),
enlivenObjectEnlivables(options),
]).then(
([objects, hydratedOptions]) =>
Expand Down
142 changes: 73 additions & 69 deletions src/shapes/Image.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// @ts-nocheck
import { getFabricDocument, getEnv } from '../env';
import type { BaseFilter } from '../filters/BaseFilter';
import { getFilterBackend } from '../filters/FilterBackend';
import { SHARED_ATTRIBUTES } from '../parser/attributes';
import { parseAttributes } from '../parser/parseAttributes';
import type { TClassProperties, TSize } from '../typedefs';
import type { TClassProperties, TCrossOrigin, TSize } from '../typedefs';
import type { Abortable } from '../typedefs';
import { uid } from '../util/internals/uid';
import { createCanvasElement } from '../util/misc/dom';
Expand All @@ -26,7 +25,10 @@ import type {
import type { ObjectEvents } from '../EventTypeDefs';
import { WebGLFilterBackend } from '../filters/WebGLFilterBackend';
import { NONE } from '../constants';
import { getDocumentFromElement } from '../util/dom_misc';
import type { CSSRules } from '../parser/typedefs';
import type { Resize } from '../filters/Resize';
import type { TCachedFabricObject } from './Object/Object';

// @todo Would be nice to have filtering code not imported directly.

Expand All @@ -41,9 +43,8 @@ interface UniqueImageProps {
cropX: number;
cropY: number;
imageSmoothing: boolean;
crossOrigin: string | null;
filters: BaseFilter[];
resizeFilter?: BaseFilter;
resizeFilter?: Resize;
}

export const imageDefaultValues: Partial<UniqueImageProps> &
Expand All @@ -58,7 +59,7 @@ export const imageDefaultValues: Partial<UniqueImageProps> &

export interface SerializedImageProps extends SerializedObjectProps {
src: string;
crossOrigin: string | null;
crossOrigin: TCrossOrigin;
filters: any[];
resizeFilter?: any;
cropX: number;
Expand Down Expand Up @@ -165,11 +166,11 @@ export class Image<
protected declare src: string;

declare filters: BaseFilter[];
declare resizeFilter: BaseFilter;
declare resizeFilter: Resize;

protected declare _element: ImageSource;
protected declare _filteredEl?: HTMLCanvasElement;
protected declare _originalElement: ImageSource;
protected declare _filteredEl: ImageSource;

static type = 'Image';

Expand Down Expand Up @@ -200,7 +201,7 @@ export class Image<
this.setElement(
typeof arg0 === 'string'
? ((
(this.canvas && getElementDocument(this.canvas.getElement())) ||
(this.canvas && getDocumentFromElement(this.canvas.getElement())) ||
Copy link
Member Author

Choose a reason for hiding this comment

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

buuuug

Copy link
Contributor

Choose a reason for hiding this comment

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

TS helps sometimes

getFabricDocument()
).getElementById(arg0) as ImageSource)
: arg0,
Expand Down Expand Up @@ -358,12 +359,12 @@ export class Image<
* of the instance
*/
_toSVG() {
const imageMarkup = [],
const imageMarkup: string[] = [],
element = this._element,
x = -this.width / 2,
y = -this.height / 2;
let svgString = [],
strokeSvg,
let svgString: string[] = [],
strokeSvg: string[] = [],
clipPath = '',
imageRendering = '';
if (!element) {
Expand All @@ -387,46 +388,30 @@ export class Image<
clipPath = ' clip-path="url(#imageCrop_' + clipPathId + ')" ';
}
if (!this.imageSmoothing) {
imageRendering = '" image-rendering="optimizeSpeed';
imageRendering = ' image-rendering="optimizeSpeed"';
}
imageMarkup.push(
'\t<image ',
'COMMON_PARTS',
'xlink:href="',
this.getSvgSrc(true),
'" x="',
x - this.cropX,
'" y="',
y - this.cropY,
// we're essentially moving origin of transformation from top/left corner to the center of the shape
// by wrapping it in container <g> element with actual transformation, then offsetting object to the top/left
// so that object's center aligns with container's left/top
'" width="',
element.width || element.naturalWidth,
'" height="',
element.height || element.naturalHeight,
imageRendering,
'"',
clipPath,
'></image>\n'
`xlink:href="${this.getSvgSrc(true)}" x="${x - this.cropX}" y="${
y - this.cropY
// we're essentially moving origin of transformation from top/left corner to the center of the shape
// by wrapping it in container <g> element with actual transformation, then offsetting object to the top/left
// so that object's center aligns with container's left/top
}" width="${
element.width || (element as HTMLImageElement).naturalWidth
}" height="${
element.height || (element as HTMLImageElement).naturalHeight
}"${imageRendering}${clipPath}></image>\n`
);

if (this.stroke || this.strokeDashArray) {
const origFill = this.fill;
this.fill = null;
strokeSvg = [
'\t<rect ',
'x="',
x,
'" y="',
y,
'" width="',
this.width,
'" height="',
this.height,
'" style="',
this.getSvgStyles(),
'"/>\n',
`\t<rect x="${x}" y="${y}" width="${this.width}" height="${
this.height
}" styles="${this.getSvgStyles()}" />\n`,
];
this.fill = origFill;
}
Expand All @@ -446,14 +431,14 @@ export class Image<
getSrc(filtered?: boolean): string {
const element = filtered ? this._element : this._originalElement;
if (element) {
if (element.toDataURL) {
return element.toDataURL();
if ((element as HTMLCanvasElement).toDataURL) {
return (element as HTMLCanvasElement).toDataURL();
}

if (this.srcFromAttribute) {
return element.getAttribute('src');
return element.getAttribute('src') || '';
} else {
return element.src;
return (element as HTMLImageElement).src;
}
} else {
return this.src || '';
Expand Down Expand Up @@ -517,7 +502,7 @@ export class Image<
this._lastScaleX = filter.scaleX = scaleX;
this._lastScaleY = filter.scaleY = scaleY;
getFilterBackend().applyFilters(
[filter],
[filter as BaseFilter],
elementToFilter,
sourceWidth,
sourceHeight,
Expand All @@ -542,29 +527,35 @@ export class Image<

if (filters.length === 0) {
this._element = this._originalElement;
this._filteredEl = null;
// this is unsafe and needs to be rethinkend
this._filteredEl = undefined;
this._filterScalingX = 1;
this._filterScalingY = 1;
return;
}

const imgElement = this._originalElement,
sourceWidth = imgElement.naturalWidth || imgElement.width,
sourceHeight = imgElement.naturalHeight || imgElement.height;
sourceWidth =
(imgElement as HTMLImageElement).naturalWidth || imgElement.width,
sourceHeight =
(imgElement as HTMLImageElement).naturalHeight || imgElement.height;

if (this._element === this._originalElement) {
// if the element is the same we need to create a new element
// if the _element a reference to _originalElement
// we need to create a new element to host the filtered pixels
const canvasEl = createCanvasElement();
canvasEl.width = sourceWidth;
canvasEl.height = sourceHeight;
this._element = canvasEl;
this._filteredEl = canvasEl;
} else {
// clear the existing element to get new filter data
// also dereference the eventual resized _element
} else if (this._filteredEl) {
// if the _element is it own element,
// and we also have a _filteredEl, then we clean up _filteredEl
// and we assign it to _element.
// in this way we invalidate the eventual old resize filtered element
this._element = this._filteredEl;
this._filteredEl
.getContext('2d')
.getContext('2d')!
.clearRect(0, 0, sourceWidth, sourceHeight);
// we also need to resize again at next renderAll, so remove saved _lastScaleX/Y
this._lastScaleX = 1;
Expand All @@ -575,7 +566,7 @@ export class Image<
this._originalElement,
sourceWidth,
sourceHeight,
this._element
this._element as HTMLCanvasElement
);
if (
this._originalElement.width !== this._element.width ||
Expand Down Expand Up @@ -605,8 +596,14 @@ export class Image<
* it will set the imageSmoothing for the draw operation
* @param {CanvasRenderingContext2D} ctx Context to render on
*/
drawCacheOnCanvas(ctx: CanvasRenderingContext2D) {
drawCacheOnCanvas(
this: TCachedFabricObject<Props, SProps, EventSpec> & {
imageSmoothing: boolean;
},
ctx: CanvasRenderingContext2D
) {
ctx.imageSmoothingEnabled = this.imageSmoothing;
// @ts-ignore ( expect-error) this line is not error free.
super.drawCacheOnCanvas(ctx);
}

Expand Down Expand Up @@ -637,8 +634,11 @@ export class Image<
// crop values cannot be lesser than 0.
cropX = Math.max(this.cropX, 0),
cropY = Math.max(this.cropY, 0),
elWidth = elementToDraw.naturalWidth || elementToDraw.width,
elHeight = elementToDraw.naturalHeight || elementToDraw.height,
elWidth =
(elementToDraw as HTMLImageElement).naturalWidth || elementToDraw.width,
elHeight =
(elementToDraw as HTMLImageElement).naturalHeight ||
elementToDraw.height,
sX = cropX * scaleX,
sY = cropY * scaleY,
// the width height cannot exceed element width/height, starting from the crop offset.
Expand Down Expand Up @@ -794,16 +794,16 @@ export class Image<
options: Abortable = {}
) {
return Promise.all([
loadImage(src, { ...options, crossOrigin }),
f && enlivenObjects(f, options),
loadImage(src!, { ...options, crossOrigin }),
f && enlivenObjects<BaseFilter>(f, options),
// TODO: redundant - handled by enlivenObjectEnlivables
rf && enlivenObjects([rf], options),
rf && enlivenObjects<BaseFilter>([rf], options),
enlivenObjectEnlivables(object, options),
]).then(([el, filters = [], [resizeFilter] = [], hydratedProps = {}]) => {
return new this(el, {
...object,
// TODO: this creates a difference between image creation and restoring from JSON
src,
crossOrigin,
Copy link
Member Author

Choose a reason for hiding this comment

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

i suspect this is the cause of some breakage in image.clone i can't explain.

filters,
resizeFilter,
...hydratedProps,
Expand All @@ -818,11 +818,14 @@ export class Image<
* @param {LoadImageOptions} [options] Options object
* @returns {Promise<Image>}
*/
static fromURL<T extends TProps<SerializedImageProps>>(
static fromURL<T extends TProps<ImageProps>>(
url: string,
options: T & LoadImageOptions = {}
{ crossOrigin = null, signal }: LoadImageOptions = {},
imageOptions: T
): Promise<Image> {
return loadImage(url, options).then((img) => new this(img, options));
return loadImage(url, { crossOrigin, signal }).then(
(img) => new this(img, imageOptions)
);
}

/**
Expand All @@ -843,10 +846,11 @@ export class Image<
this.ATTRIBUTE_NAMES,
cssRules
);
return this.fromURL(parsedAttributes['xlink:href'], {
...options,
...parsedAttributes,
}).catch((err) => {
return this.fromURL(
parsedAttributes['xlink:href'],
options,
parsedAttributes
).catch((err) => {
console.log(err);
return null;
});
Expand Down
8 changes: 6 additions & 2 deletions src/shapes/Object/Object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ import type { ObjectProps } from './types/ObjectProps';
import type { TProps } from './types';
import { getEnv } from '../../env';

export type TCachedFabricObject = FabricObject &
export type TCachedFabricObject<
Props extends TProps<ObjectProps> = Partial<ObjectProps>,
SProps extends SerializedObjectProps = SerializedObjectProps,
EventSpec extends ObjectEvents = ObjectEvents
> = FabricObject<Props, SProps, EventSpec> &
Required<
Pick<
FabricObject,
FabricObject<Props, SProps, EventSpec>,
| 'zoomX'
| 'zoomY'
| '_cacheCanvas'
Expand Down
Loading