Skip to content

Comments

Feat: Extend platform abstraction#746

Open
wouterlucas wants to merge 13 commits intomainfrom
feat/platform-abstraction
Open

Feat: Extend platform abstraction#746
wouterlucas wants to merge 13 commits intomainfrom
feat/platform-abstraction

Conversation

@wouterlucas
Copy link
Contributor

@wouterlucas wouterlucas commented Feb 18, 2026

Move Image creation and GL wrapper to Platform.

This PR changes:

  • createImageBitmap support to Platform
  • Image fetch to Platform (by default using XMLHttpRequest)
  • ImageWorker handling to Platform
  • GLWrapper to Platform
  • Similar, support for TX compression and SVG handling has been moved to Platform

Removed:

  • createImageBitmap auto detection - this needs to be done by Platform
  • Image Fallback handling - this is done by Platform

NOTE: BREAKING CHANGES
This requires a different Platform import / init to make it work.

The concept is you create a Platform that matches the functionality the platform exposes. I.e. if the Platform does not support createImageBitmap or a less version of createImageBitmap you should extend the platform to match that.

@estobbart for ION you can create or upstream an ION specific platform (or we can create one) that:

  • Replace loadImage and createImage in Platform (possibly Fetch too?)
  • Replace the respective GL function

Todo:

  • Expose platforms in exports
  • Create Legacy Platform for non-createImageBitmap platform
  • Create Chrome 50 createImageBitmap platform
  • Create fetch supported platform

@wouterlucas wouterlucas added breaking change! This issue / PR may require downstream dependencies to make changes to retain existing functionality ION labels Feb 18, 2026
@wouterlucas wouterlucas force-pushed the feat/platform-abstraction branch from 26467af to dd3a25e Compare February 19, 2026 15:49
@wouterlucas wouterlucas marked this pull request as ready for review February 20, 2026 15:10
Copy link
Collaborator

@estobbart estobbart left a comment

Choose a reason for hiding this comment

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

Couple of questions..
L2 had a force draw capability so that every raf would draw, is that possible here?

const gl =
// TODO: Remove this assertion once this issue is fixed in TypeScript
// https://github.com/microsoft/TypeScript/issues/53614
(canvas.getContext(forceWebGL2 ? 'webgl2' : 'webgl', config) ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

this bit was confusing.. when should it ask canvas for a context vs the platform layer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just to force the context to webgl2 if that's supported based on config - that is added for when devices support both but we want to force it

Copy link
Collaborator

Choose a reason for hiding this comment

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

why not get the canvas from the platform?

sh ?? 0,
options,
override fetch(url: string): Promise<Blob> {
return new Promise((resolve, reject) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure I understand the intent w/ wrapping fetch here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added that because not all devices support fetch API and require some specifics around using XMLHttpRequest - some older Smart TVs give us trouble with local URLs or other specifics, to allow Platform layers to modify the requests if needed,

Copy link
Collaborator

Choose a reason for hiding this comment

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

worth a check first?

return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest();
xhr.responseType = '';
xhr.onreadystatechange = function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO loadend would be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can look into that - but as said some devices are really picky about this, especially older Samsung devices

canvas.width = deviceLogicalWidth * devicePhysicalPixelRatio;
canvas.height = deviceLogicalHeight * devicePhysicalPixelRatio;
// set main canvas reference and size
platform.canvas = this.canvas;
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe canvas should be one of the platform settings (along with force & num workers).. then the platform knows if it needs to create a canvas or not. Then platform createCanvas could be getCanvas.. making it more clear if it should be creating, and/or what the ownership is supposed to be of the canvas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah struggling with that a little bit, there are multiple Canvas being used in certain cases:

  • Main drawing canvas
  • Font generation canvas
  • SVG canvas (if used)

So I initially just had platform set itself, but then got stuck with a 1x1px Canvas used for fonts with clipping calculations.

Its not just the platform, its also the font engine that determines whether it is 1 or more Canvas'es used. This was easier, but open to improve it!

Copy link
Collaborator

Choose a reason for hiding this comment

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

having the property and the method is the confusing piece to me.. overall be good to understand the canvas usage/lifecycle that you described above.
my recommendation would be:

    const platform = new (settings.platform as any)({
      numImageWorkers: settings.numImageWorkers,
      forceWebGL2: settings.forceWebGL2,
      canvas: settings.canvas
    });

    // platform layer responsible for providing the canvas from settings or making one if not provided.
    this.canvas = platform.getCanvas();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change! This issue / PR may require downstream dependencies to make changes to retain existing functionality ION

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants