League of Extraordinary FoundryVTT Developers•2y ago

Sequencer Firefox Performance fixes

221 Replies

Alright so let me go through the high level reproduction steps: 1. Open up a game of Foundry in Firefox. 2. Add this effect to about however many tokens lag you, for me it's about a dozen:

for (const token of game.canvas.tokens.controlled) {
    new Sequence()
        .effect()
            .file("jb2a.wind_stream.white", true)
            .attachTo(token)
            .persist()
        .play();
}

for (const token of game.canvas.tokens.controlled) {
    new Sequence()
        .effect()
            .file("jb2a.wind_stream.white", true)
            .attachTo(token)
            .persist()
        .play();
}

Wasp•2y ago

https://raw.githubusercontent.com/fantasycalendar/FoundryVTT-Sequencer/next/module.json Have ya tried this version? It's the upcoming 3.0.0 version, should have some improvements, but I'm still open to hearing what you've found!

LukeAbbyOP•2y ago

I'll pull it up again but I'll note that there's no prototype token persistence

Wasp•2y ago

Ah, fair

LukeAbbyOP•2y ago

The main change I made was this:

const videos = {};
async function get_video_texture(inSrc, inBlob) {
  console.log("GOT BLOB", inBlob);
  if (inSrc in videos) {
    console.log("VIDEO IS CACHED");
    return videos[inSrc];
  }

  const texturePromise = new Promise(async (resolve) => {
    const video = document.createElement("video");
    video.preload = "auto";
    video.crossOrigin = "anonymous";
    video.controls = true;
    video.autoplay = false;
    video.autoload = true;
    video.muted = true;
    video.src = URL.createObjectURL(inBlob);

    let canplay = true;
    video.oncanplay = async () => {
      if (!canplay)
        return;
      canplay = false;
      video.height = video.videoHeight;
      video.width = video.videoWidth;
      const baseTexture = PIXI.BaseTexture.from(video, { resourceOptions: { autoPlay: false } });
      if (game.settings.get(CONSTANTS.MODULE_NAME, "enable-pixi-fix")) {
        baseTexture.alphaMode = PIXI.ALPHA_MODES.PREMULTIPLIED_ALPHA;
      }
      const texture = new PIXI.Texture(baseTexture);
      resolve(texture);
    };
    video.onerror = () => {
        URL.revokeObjectURL(video.src);
        delete videos[inSrc];
        reject();
    };
  });
  videos[inSrc] = texturePromise;

  return texturePromise;
}

const videos = {};
async function get_video_texture(inSrc, inBlob) {
  console.log("GOT BLOB", inBlob);
  if (inSrc in videos) {
    console.log("VIDEO IS CACHED");
    return videos[inSrc];
  }

  const texturePromise = new Promise(async (resolve) => {
    const video = document.createElement("video");
    video.preload = "auto";
    video.crossOrigin = "anonymous";
    video.controls = true;
    video.autoplay = false;
    video.autoload = true;
    video.muted = true;
    video.src = URL.createObjectURL(inBlob);

    let canplay = true;
    video.oncanplay = async () => {
      if (!canplay)
        return;
      canplay = false;
      video.height = video.videoHeight;
      video.width = video.videoWidth;
      const baseTexture = PIXI.BaseTexture.from(video, { resourceOptions: { autoPlay: false } });
      if (game.settings.get(CONSTANTS.MODULE_NAME, "enable-pixi-fix")) {
        baseTexture.alphaMode = PIXI.ALPHA_MODES.PREMULTIPLIED_ALPHA;
      }
      const texture = new PIXI.Texture(baseTexture);
      resolve(texture);
    };
    video.onerror = () => {
        URL.revokeObjectURL(video.src);
        delete videos[inSrc];
        reject();
    };
  });
  videos[inSrc] = texturePromise;

  return texturePromise;
}

this is the compiled js I added everything related to videos it seems like Chrome does this automatically or something, it changes NOTHING for Chrome but on Firefox when I have 10 effects it seems to make 10 corresponding videos and upload them to the GPU constantly which eventually strains the GPU when it's a large enough effect (over a MB in this case)

Wasp•2y ago

The main problem with that is that if all of them share the same video, they all have the same playback So if one is removed/paused/loops or needs have a different offset to its playback, that's not possible

LukeAbbyOP•2y ago

hmm, understood I understand this isn't a general fix then, can you see if you reproduce this by pulling up Firefox though?

Wasp•2y ago

Firefox is generally terrible in foundry from what I know, due to it being largely out of sync with the chromium-verse Pretty much everyone in the mothership recommends to play on anything but Firefox Like, if you place multiple animated tiles on the scene, do you get the same issues?

LukeAbbyOP•2y ago

Fair, I just realised that it was a Sequencer "bug" though once I tried reproducing in pure PIXI here's what I tried initially

function start() {
    const app = createApp();
    
    const windStreamPath = Sequencer.Database.getEntry("jb2a.wind_stream.white").file;
    
    const windStreamTexture = PIXI.Texture.from(windStreamPath);
    windStreamTexture.baseTexture.resource.source.loop = true;

    const positions = [100, 200, 300, 400, 500];
    for (let i = 0; i < 100; i++) {
        for (const x of positions) {
            for (const y of positions) {
                addSprite(app, windStreamTexture, x + Math.floor(Math.random() * 10), y + Math.floor(Math.random() * 10));
            }
        }
    }
}

// Basically taken frome Sequencer's effects-layer.js and/or ui-layer.js
function createApp() {
    const canvas = document.createElement("canvas");
    canvas.id = "test-layer";

    canvas.style.cssText = `
        position:absolute;
        touch-action: none;
        pointer-events: none;
        width:100%;
        height:100%;
        z-index:0.1;
        padding: 0;
        margin: 0;
    `;

    document.body.appendChild(canvas);

    const app = new PIXI.Application({
        width: window.innerWidth,
        height: window.innerHeight,
        view: canvas,
        antialias: true,
        backgroundAlpha: 0.0,
        sharedTicker: true
    });

    app.resizeTo = window;

    globalThis.__PIXI_APP__ = app; // Allow PIXI extension to see the app.
    
    document.body.appendChild(app.view);

    return app;
}

function addSprite(app, texture, x, y) {
    const windStreamSprite = new PIXI.Sprite(texture);
    windStreamSprite.x = x;
    windStreamSprite.y = y;
    windStreamSprite.loop = true;
    
    app.stage.addChild(windStreamSprite);
}

function startFPS() {
    const ticker = new PIXI.Ticker();
    let _lastTime = new Date().getTime();
    let _timeValues = [];
    ticker.add(() => {
        const currentTime = new Date().getTime();
        _timeValues.push(1000 / (currentTime - _lastTime));

        if (_timeValues.length === 30) {
            let total = 0;
            for (let i = 0; i < 30; i++) {
                total += _timeValues[i];
            }

            console.log(total / 30);

            _timeValues.length = 0;
        }

        _lastTime = currentTime;
    });
    ticker.start();
}

start();
startFPS();

function start() {
    const app = createApp();
    
    const windStreamPath = Sequencer.Database.getEntry("jb2a.wind_stream.white").file;
    
    const windStreamTexture = PIXI.Texture.from(windStreamPath);
    windStreamTexture.baseTexture.resource.source.loop = true;

    const positions = [100, 200, 300, 400, 500];
    for (let i = 0; i < 100; i++) {
        for (const x of positions) {
            for (const y of positions) {
                addSprite(app, windStreamTexture, x + Math.floor(Math.random() * 10), y + Math.floor(Math.random() * 10));
            }
        }
    }
}

// Basically taken frome Sequencer's effects-layer.js and/or ui-layer.js
function createApp() {
    const canvas = document.createElement("canvas");
    canvas.id = "test-layer";

    canvas.style.cssText = `
        position:absolute;
        touch-action: none;
        pointer-events: none;
        width:100%;
        height:100%;
        z-index:0.1;
        padding: 0;
        margin: 0;
    `;

    document.body.appendChild(canvas);

    const app = new PIXI.Application({
        width: window.innerWidth,
        height: window.innerHeight,
        view: canvas,
        antialias: true,
        backgroundAlpha: 0.0,
        sharedTicker: true
    });

    app.resizeTo = window;

    globalThis.__PIXI_APP__ = app; // Allow PIXI extension to see the app.
    
    document.body.appendChild(app.view);

    return app;
}

function addSprite(app, texture, x, y) {
    const windStreamSprite = new PIXI.Sprite(texture);
    windStreamSprite.x = x;
    windStreamSprite.y = y;
    windStreamSprite.loop = true;
    
    app.stage.addChild(windStreamSprite);
}

function startFPS() {
    const ticker = new PIXI.Ticker();
    let _lastTime = new Date().getTime();
    let _timeValues = [];
    ticker.add(() => {
        const currentTime = new Date().getTime();
        _timeValues.push(1000 / (currentTime - _lastTime));

        if (_timeValues.length === 30) {
            let total = 0;
            for (let i = 0; i < 30; i++) {
                total += _timeValues[i];
            }

            console.log(total / 30);

            _timeValues.length = 0;
        }

        _lastTime = currentTime;
    });
    ticker.start();
}

start();
startFPS();

sorry for the length but the key stuff is that this DOESN'T reproduce the problem and I realised it's not reproducing it because it's reusing the texture for every sprite

Wasp•2y ago

Indeed

LukeAbbyOP•2y ago

even though it's making 100*5*5 sprites it doesn't really lag at all yeah

Wasp•2y ago

Essentially, each video texture is a whole html video tag and browser tabs can only generally handle a maximum of 70, that's the hard coded limit but most can at most handle 5-10, depending on the underlying system and browser

LukeAbbyOP•2y ago

well my player starts getting lag around that area (5-10) yeah and I think it's because it's a large texture it's scaled down to the player token so it's unnecessarily high resolution

Wasp•2y ago

might be a good idea to request a scaled down version from the JB2A guys

LukeAbbyOP•2y ago

yeah, I'll look into something like that would you accept a fix if I were to PR one that still kept all the features of current Sequencer to cache textures in a way that plays nice with firefox? I imagine it'd help with the chrome video limit as well. I can suggest my players not use Firefox but I imagine that'd annoy them

Wasp•2y ago

hyup Do lemme know if animated tiles behave the same way In theory they should, if they don't, then we have a different problem

LukeAbbyOP•2y ago

lemme take a look doesn't lag on Chrome lags on Firefox so yeah probably a similar issue So animated tiles, will I have to submit a patch for this separately? Is it all core code? honestly this is the first big issue I've run into using Firefox, my players wouldn't listen to me if I told them to switch browsers for the game though would it be possible to automatically preprocess/save the video into a lower size and resolution? I suspect the answer is no buuut if there's a way like maybe a cache() method that can be opted into so that simple effects don't get cached but a downscale of a large video can, I have no idea what to call it

Wasp•2y ago

Each frame would have to be rendered to a separate canvas, put together into a new video or an animated sprite: https://pixijs.download/dev/docs/PIXI.AnimatedSprite.html I could try it, but loading times might become rough and caching all of the images in 24fps videos that can be between 5-10 seconds long seem... risky

LukeAbbyOP•2y ago

ouch that'd be annoying

Wasp•2y ago

they'd only have to load once, but yeah Worth a shot through, potentially

LukeAbbyOP•2y ago

right I'll look into that after I look into caching the video textures dunno if it's a landable optimisation What are all the things I should look out for breaking while trying to optimise? I've identified delay and playbackRate as things simply naively caching the video breaks, is there anything else? Also is there any particular reason you're not using PIXI.Texture.from(inSrc)? I think it'd be able to replace nearly all of get_video_texture. You make the video html element yourself and setup a bunch of things yourself and I THINK PIXI.Texture.from can handle most of it with just a string input. I bet you have some specific reasons I just want to hear them.

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

oh nice! Did you move over to that, Wasp? actually I dunno if their 3.0.0 version is v11 ready

Unknown User•2y ago

Message Not Public

Wasp•2y ago

Fantastic!

LukeAbbyOP•2y ago

nice! Thank you! Also wdym video assets aren't supported? As in videos in PIXI.Assets?

Wasp•2y ago

From what I know, PIXI.Texture.from loads the URL every time fresh, without caching? I think? Not sure.

LukeAbbyOP•2y ago

BaseTextures are automatically cached, so that calling PIXI.Texture.from() repeatedly for the same URL returns the same BaseTexture each time. Destroying a BaseTexture frees the image data associated with it.

Wasp•2y ago

get_video_texture is a holdover from v9 when source video texture was replicated to all tiles

LukeAbbyOP•2y ago

https://pixijs.io/guides/basics/textures.html

PixiJS Guides

The holy guide for PixiJS developers

Wasp•2y ago

ah nice.

LukeAbbyOP•2y ago

might not be true for current Foundry PIXI version but it says this much here yeah

Wasp•2y ago

If I try to create multiple textures with PIXI.Texture.from(inSrc), it doesn't allow me to create multiple effects with the same source since the texture shares the video across the baseTexture

LukeAbbyOP•2y ago

it's because you're modifying the base texture, yeah I'm hoping to figure out how to have multiple videos with different offsets etc. with the same BaseTexture If I can't I might try a caching strategy where it's url + playback speed + delay together to cache the texture, maybe opt-in, since it'd solve this case still

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

lame, do you have a reference for that so I can know for sure? It seems like Chrome will still handle multiple videos better than Firefox, it probably caches the video internally while Firefox will actually upload to the GPU as many times as the number of BaseTexture

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

it's only a few seconds video and having to decode to that frame once isn't a big deal imo, it might be a big deal if it's like really long video but I'm getting really large lag with about only a dozen video sprites without caching of the BaseTexture. I think the effects are generally short enough to not be a big deal to do BUT I'll certainly keep that concern in mind. One of my players gets lag with about 3-5 so they've turned effects off entirely but they're totally fine with 1, it's just a common effect. so yeah if the video API supports per texture seeking of an underlying video etc. that's what I'd look for

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

perhaps? Though I'd imagine even as a video element they have to seek and take some time effects taking a small moment to load is probably fine (?)

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

ahh I see what you're saying. In my mind it's loaded into memory, I think the whole thing gets uploaded to the GPU all in memory in Firefox but there's probably cases where they doesn't happen like if there was an hour long video and you used it for a source for two textures and seeked back and forth you'd potentially trash things if it's trying to read through the whole video every single frame though maybe they automatically remember the seek offset and read it in parallel, like have two readers of the file only when required? I'll definitely try to cache them if they're at the same offset though since that'll solve things for this specific case I'm really only considering offsets trying to solve the general case Hey @wasp quick question, I've potentially found a solution that'd handle this case but it requires loading everything in the video into memory. I think this might be acceptable because it looks like that's already the case for videos but I want to make sure this isn't a deal breaker quickly, first. I think it's all already loaded into memory because of this code:

      const blob = await fetch(inSrc, {
        mode: "cors",
        credentials: "same-origin",
      })
        .then((r) => r.blob())
        .catch((err) => {
          console.error(err);
        });

      const blob = await fetch(inSrc, {
        mode: "cors",
        credentials: "same-origin",
      })
        .then((r) => r.blob())
        .catch((err) => {
          console.error(err);
        });

Wasp•2y ago

I'm already keeping the blob in memory for video reconstruction later

LukeAbbyOP•2y ago

okay sweet

Wasp•2y ago

const blob = await fetch(inSrc, { mode: "cors", credentials: "same-origin" })
  .then(r => r.blob())
  .catch(err => {
    console.error(err)
  });

const blob = await fetch(inSrc, { mode: "cors", credentials: "same-origin" })
  .then(r => r.blob())
  .catch(err => {
    console.error(err)
  });

and then:

this._videos[inSrc] = {
  blob,
  lastUsed: (+new Date())
};

this._videos[inSrc] = {
  blob,
  lastUsed: (+new Date())
};

LukeAbbyOP•2y ago

Rightyo, I just wanted to double check since I haven't dealt with blobs much at all I know they're just conceptually an array of bytes

Wasp•2y ago

it's the raw binary data of the file, yeah

LukeAbbyOP•2y ago

but I don't know if they're lazy or not

Wasp•2y ago

well, this line should take care of that: https://github.com/fantasycalendar/FoundryVTT-Sequencer/blob/master/scripts/modules/sequencer-file-cache.js#L12

LukeAbbyOP•2y ago

nono I meant Blobs as in they're a read handle and doesn't load everything if it's too large but upon further inspection Response.blob() says it loads it all:

The blob() method of the Response interface takes a Response stream and reads it to completion. It returns a promise that resolves with a Blob.

guess there's no hour long video effects possible in sequencer not that I have any idea why you'd do that

Wasp•2y ago

I don't... know what you mean? There's an await in front of the fetch then blob?

LukeAbbyOP•2y ago

I was trying to surmise if the code is analogous to:

fs.readFileSync(...);

fs.readFileSync(...);

Which loads it all into memory upfront OR

fileHandle.createReadStream();

fileHandle.createReadStream();

which just gives you a stream that's lazily read as you go it sounds like it's the first one, I just couldn't tell that easily whether fetch + .blob()'s semantics were like the first or the second

Wasp•2y ago

Yeah, I think it is, if you inspect the blob that is returned, it is indeed a bytearray

LukeAbbyOP•2y ago

right I saw that so I suspected it was loaded into memory I just didn't want the assumption to come back and bite me say if they automatically load 1 MiB upfront and load the rest later Also follow up question, do you think it'd be a good idea to do URL.revokeObjectURL when a BaseTexture for a video is destroy'd (used when garbage collecting a BaseTexture after it's no longer reachable on the stage or just whenever you want) I ask because you mentioned that the number of video elements you're allowed to have is limited, does PIXI know to "free" the video html tag when a BaseTexture is destroyed? If so this is not necessary and already done for you but it might be a nice win if PIXI doesn't.

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

Yeah okay I think they should be revoked then. on destroy Something like:

async function get_video_texture(inBlob) {

  return new Promise(async (resolve) => {

    const video = document.createElement("video");
    ...

    let canplay = true;
    video.oncanplay = async () => {
      ...

      const baseTexture = PIXI.BaseTexture.from(video, { resourceOptions: { autoPlay: false } });
      const destroy = baseTexture.destroy.bind(baseTexture);
      baseTexture.destroy = function() {
          URL.revokeObjectURL(video.src);
          return destroy();
      }

      if (game.settings.get(CONSTANTS.MODULE_NAME, "enable-pixi-fix")) {
        baseTexture.alphaMode = PIXI.ALPHA_MODES.PREMULTIPLIED_ALPHA;
      }

      const texture = new PIXI.Texture(baseTexture);

      resolve(texture);
    };

    video.onerror = () => {
      URL.revokeObjectURL(video.src);
      reject();
    };

  });

}

async function get_video_texture(inBlob) {

  return new Promise(async (resolve) => {

    const video = document.createElement("video");
    ...

    let canplay = true;
    video.oncanplay = async () => {
      ...

      const baseTexture = PIXI.BaseTexture.from(video, { resourceOptions: { autoPlay: false } });
      const destroy = baseTexture.destroy.bind(baseTexture);
      baseTexture.destroy = function() {
          URL.revokeObjectURL(video.src);
          return destroy();
      }

      if (game.settings.get(CONSTANTS.MODULE_NAME, "enable-pixi-fix")) {
        baseTexture.alphaMode = PIXI.ALPHA_MODES.PREMULTIPLIED_ALPHA;
      }

      const texture = new PIXI.Texture(baseTexture);

      resolve(texture);
    };

    video.onerror = () => {
      URL.revokeObjectURL(video.src);
      reject();
    };

  });

}

would do it I think, though it's hacky, I dunno if there's an "ondestroy" event since I don't think it'd trigger onerror when the texture is destroyed

Unknown User•2y ago

Message Not Public

Wasp•2y ago

(sequencer is open source pls help)

LukeAbbyOP•2y ago

Well I'll pr the first easier change soon ™️, whenever I get a chance to check things out. Just got finished with a session That'll be the video URL related stuff Scratch that, I think I've found out a way to avoid using revokeObjectURL etc. entirely but I'll make it a part of the larger PR that aims to fix Firefox performance issues.

Wasp•2y ago

@lukeabby @EBER Good news! I've implemented support for AnimatedSprites! https://cdn.discordapp.com/attachments/772596237605011468/1096096329668767815/warpspeed.mp4

LukeAbbyOP•2y ago

Sweet, do you think it'll relate to the performance issues at all or nah?

Wasp•2y ago

It's 100% related to the webm playback These are AnimatedSprites with about 56 Textures each Doesn't even break a sweat

LukeAbbyOP•2y ago

Are you making an Animated sprite by extracting each frame of the WebM? I actually wrote similar code, I think you might've seem me ask about it in the PIXIJS server The main thing I was trying to profile was increased memory consumption

Wasp•2y ago

I have not no, this is the raw webp's created by JB2A

LukeAbbyOP•2y ago

Ah okay sweet, ideal even

Wasp•2y ago

Since it's 56 textures that cost about 50-100 kb each, it's not a huge cost and they get reused across all AnimatedSprites

LukeAbbyOP•2y ago

Yup. Is that 56 network requests though?

Wasp•2y ago

Yep, unfortunately Pretty quick though

LukeAbbyOP•2y ago

Maybe you could ask JB2a to bundle a spritesheet It's not a huge deal TODAY

Wasp•2y ago

Spritesheets end up taking about 10mb

LukeAbbyOP•2y ago

But 56 is still rather large

Wasp•2y ago

It's about 10-20x the size of the webm and has a loading time even locally unfortunately

LukeAbbyOP•2y ago

Hmmmm

Wasp•2y ago

The separate textures load super quick They seem to load async, so it's alright

LukeAbbyOP•2y ago

Try it with the network tab simulating high ping If it's faster as 56 requests in both cases, so be it as that keeps it simple, but high ping would be the case I'd worry about

Wasp•2y ago

It seems to only load a single texture :/ Odd. Oh nevermind sec

LukeAbbyOP•2y ago

Did you like only remember to wait for the first texture?

Wasp•2y ago

Nah I wasn't loaded into the right world

LukeAbbyOP•2y ago

Ahhh

Wasp•2y ago

What do you reckon the average download speed and latency is? The good part is that it can play the effect partially before all of its frames has been loaded As opposed to a flipbook, taking the same amount of time to load as all of them, if not longer

Wasp•2y ago

5mb/s download, 200ms latency

LukeAbbyOP•2y ago

Hmm hard to say because Foundry can be self-hosted One of my players has genuinely those kinds of stats in terms of internet speed Hey, Wasp can you tell me a bit about this code in canvas-effect.js?:

    if (this._file?.markers && this._startTime === 0 && _endTime === this.video.duration) {
      this._animationTimes.loopStart = this._file.markers.loop.start / 1000;
      this._animationTimes.loopEnd = this._file.markers.loop.end / 1000;
      this._animationTimes.forcedEnd = this._file.markers.forcedEnd / 1000;
    }

    if (this._file?.markers && this._startTime === 0 && _endTime === this.video.duration) {
      this._animationTimes.loopStart = this._file.markers.loop.start / 1000;
      this._animationTimes.loopEnd = this._file.markers.loop.end / 1000;
      this._animationTimes.forcedEnd = this._file.markers.forcedEnd / 1000;
    }

Specifically can you tell me what type this._file is expected to be when markers is set? I can't find it on HTMLVideoElement (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video), SequencerFileBase, SequencerFile, or as an entry, or even in PIXI's docs anywhere and as far as I can tell those are the only things that this._file can be from. I can't even find markers anywhere else in code, I can find it in Sequencer's documentation at database-basics.md but it's an object with _markers NOT markers. Is the documentation just out of date? Also would you accept a PR to apply your formatter, that is the .prettierrc you already have? It seems to not be applied to a lot of places in the code base and I've had to be careful to not reformat existing code. I can add it to the CI as a requirement if that'd help. (@wasp ping for visibility, although I can avoid pings if you think you'll see messages here just fine)

Wasp•2y ago

I'll respond to this when I get home, cheers

LukeAbbyOP•2y ago

No problem

Wasp•2y ago

Basically, files in the sequencer database can have metadata assigned to them, and those get expressed as properties in the this._file property on the canvas effect, which is indeed a SequencerFile. This metadata is entirely optional and is only defined by people who register entries in the sequencer database. I hope that explains it. And go wild

LukeAbbyOP•2y ago

The database documentation mentions a _markers property in the database but not a markers Should the documentation or the code be corrected? What branch do you want PRs on? I've been experimenting on stable for the Firefox performance fixes But I want to put any pr I end up making to be off of your ideal branch

Wasp•2y ago

Master please The _ denotes metadata, so it can be whatever they want it to be

LukeAbbyOP•2y ago

Ahhhh

Wasp•2y ago

Markers is just explicitly supported in behaviour too

LukeAbbyOP•2y ago

Goootchaaa I woulda put it under a metadata property but it makes sense now Gotcha, I won't make a PR off of next then Prettier is currently configured to do 4 spaces but most of the repo is 2 spaces. Do you want me to swap .prettierrc to be 2 spaces?

Wasp•2y ago

2 please, with tabs

LukeAbbyOP•2y ago

tabWidth: 2 you mean?

{
  "trailingComma": "es5",
  "tabWidth": 2,
  "semi": true,
  "singleQuote": false
}

{
  "trailingComma": "es5",
  "tabWidth": 2,
  "semi": true,
  "singleQuote": false
}

here's the current config, I only changed tabWidth

Wasp•2y ago

Yeah That's good

LukeAbbyOP•2y ago

I notice that you do await this._contextLostCallback(); but that the function isn't returning a promise or async:

  _contextLostCallback() {
    if (this.isSourceTemporary) {
      this._ticker.add(() => {
        if (this.isSourceDestroyed) {
          this._ticker.stop();
          this._source = this.sourcePosition;
          SequencerEffectManager.endEffects({ effects: this });
        }
      });
    }
    if (this.isTargetTemporary) {
      this._ticker.add(() => {
        if (this.isTargetDestroyed) {
          this._ticker.stop();
          this._target = this.targetPosition;
          SequencerEffectManager.endEffects({ effects: this });
        }
      });
    }
  }

  _contextLostCallback() {
    if (this.isSourceTemporary) {
      this._ticker.add(() => {
        if (this.isSourceDestroyed) {
          this._ticker.stop();
          this._source = this.sourcePosition;
          SequencerEffectManager.endEffects({ effects: this });
        }
      });
    }
    if (this.isTargetTemporary) {
      this._ticker.add(() => {
        if (this.isTargetDestroyed) {
          this._ticker.stop();
          this._target = this.targetPosition;
          SequencerEffectManager.endEffects({ effects: this });
        }
      });
    }
  }

endEffects IS async so if you want to wait for that to be run, you could do something like this:

  async _contextLostCallback() {
    if (this.isSourceTemporary) {
      await new Promise((resolve) => {
        this._ticker.add(() => {
          if (this.isSourceDestroyed) {
            this._ticker.stop();
            this._source = this.sourcePosition;
            SequencerEffectManager.endEffects({ effects: this }).then(resolve);
          }
        });
      });
    }
    if (this.isTargetTemporary) {
      await new Promise((resolve) => {
        this._ticker.add(() => {
          if (this.isTargetDestroyed) {
            this._ticker.stop();
            this._target = this.targetPosition;
            SequencerEffectManager.endEffects({ effects: this }).then(resolve);
          }
        });
      });
    }
  }

  async _contextLostCallback() {
    if (this.isSourceTemporary) {
      await new Promise((resolve) => {
        this._ticker.add(() => {
          if (this.isSourceDestroyed) {
            this._ticker.stop();
            this._source = this.sourcePosition;
            SequencerEffectManager.endEffects({ effects: this }).then(resolve);
          }
        });
      });
    }
    if (this.isTargetTemporary) {
      await new Promise((resolve) => {
        this._ticker.add(() => {
          if (this.isTargetDestroyed) {
            this._ticker.stop();
            this._target = this.targetPosition;
            SequencerEffectManager.endEffects({ effects: this }).then(resolve);
          }
        });
      });
    }
  }

I found a behaviour that's probably a bug: An effect with a moving video can be cut off, it's most obvious with something like this:

new Sequence()
  .effect()
    .file("jb2a.wind_stream.white")
    .fadeOut(25000)
    .atLocation({ x: 1000, y: 1000 })
    .moveTowards({ x: 0, y: 0 })
  .play();

new Sequence()
  .effect()
    .file("jb2a.wind_stream.white")
    .fadeOut(25000)
    .atLocation({ x: 1000, y: 1000 })
    .moveTowards({ x: 0, y: 0 })
  .play();

It moves pretty quickly to x=0, y=0 and then expires before the fade out finishes. If you change it to an image like icons/svg/cowled.svg the movement will instead be slowed down to take 25s. I chose 25s because it's long enough to easily tell whether it's being respected or not. By comparison movement will slow down for the actual video's duration:

new Sequence()
  .effect()
    .file("jb2a.wind_stream.white")
    .atLocation({ x: 1, y: 1})
    .moveTowards({ x: 0, y: 0 })
  .play();

new Sequence()
  .effect()
    .file("jb2a.wind_stream.white")
    .atLocation({ x: 1, y: 1})
    .moveTowards({ x: 0, y: 0 })
  .play();

Will obviously wait for the video to finish playing before expiring. The fix is really simple, in _calculateDuration the line } else if (!this.data.duration && !this.video) { needs to be if (!this.data.duration && !this.video?.duration) { and then this._animationDuration = Math.max(fadeDuration, ...); needs to be this._animationDuration = Math.max(this._animationDuration, fadeDuration, scaleDuration, ...); The problem is basically when videoDuration < Math.max(fadeDuration, scaleDuration, ...) and durationFromSpeed > this.data.duration... actually that's not that clear...

Wasp•2y ago

Fadeout is bugged in the current live version yes Thanks for the find! And this is probably an oversight, it shouldn't await that since that's just a static listener in case the source/target of the effect is a local preview template Hm, maybe it would be better to make that pull request off of the Svelte branch, considering the amount of changes 😅 I was out and about last night and I didn't consider the large shift I had made in that branch My apologies

LukeAbbyOP•2y ago

Sure fortunately it's literally just calling a tool so it didn't take too long to put together. I'll make the commit there. For my performance pr should I put it on the Svelte branch too?

Wasp•2y ago

Might be good, yeah. There's been a lot of restructuring to support the flipbook assets - and it's based on the svelte workflow, if you've ever worked with that? You'll need to symlink/put the folder into your data/modules folder, then run npm install, npm run build, and npm run dev to get started

LukeAbbyOP•2y ago

flipbook assets? I'm not too concerned about build as long as it's speedy, vite uses esbuild under the surface so it should be fast right? Well after the first build, once dev comes into play Also is eslint meant to be there, it doesn't really have a config to work off of But it's in the module.json

Wasp•2y ago

Yeah it's vite, pretty rapid

LukeAbbyOP•2y ago

I'm just going to edit the vite config locally since it's not a big deal but I'll note that hard coding as localhost:30000 doesn't work for me

Wasp•2y ago

all good, yeah maybe an .env file would be useful

LukeAbbyOP•2y ago

Yeah or a real ENV file, however you see fit. For now I'll just edit and make sure I don't commit. Anyways PR should be ready I rebased it onto Svelte now There is a 'bug' though in that it's trying to run ESLInt Would you like me to remove ESLint from lint-staged (it detected it automatically) or would you like to set up ESLint?

Wasp•2y ago

Nah, it's fine, we can remove it, though I didn't see your last message before I merged it 😄

LukeAbbyOP•2y ago

Ah oops... I'll sneak in a tinnny PR then Created the PR

Wasp•2y ago

Thanks! So if I understood it correctly, the linter will run on commit now?

LukeAbbyOP•2y ago

Yeah, you need to run npm install, which'll run npm run prepare for you automatically, which'll run husky install and that'll set up a git pre-commit hook so when you do git commit -m "Foo" it'll automatically apply Prettier to the files you're committing. All that sounds unwieldy but again, you just have to run npm install and it'll be pretty transparent to you. If you're sure you want to commit something without lint-staged you can do git commit -m "Foo" --no-verify but I'd suggest against it since it's pretty fast and helps keep style consistent Feel free to modify how this works however you want, I just noticed prettier was already there but not getting applied. It's primarily your repo so how it's maintained should be how you want it to work. I can help out with it too if you'd like ofc

LukeAbbyOP•2y ago

BTW how would you feel about using something like this: https://github.com/zz85/timeliner For a visual editor of Sequencer. I will say upfront that I'd be willing to help out, maybe even PR it entirely, but it'll probably add maintenance cost so I wouldn't want to get started if you don't want something like this. I think it'd be pretty cool though, personally. I tried figuring out how to edit an existing effect using the layer tools but I could only figure out how to move it and delete it. I also thought of the timeline stuff when I was digging into how animations are timed.

GitHub

GitHub - zz85/timeliner: simple javascript timeline library for ani...

simple javascript timeline library for animation and prototyping - GitHub - zz85/timeliner: simple javascript timeline library for animation and prototyping

Wasp•2y ago

Yeah, that's been a feature that has been requested a couple of times We'd also have to refactor the Sequencer Animation Engine to accept this kind of data for playback

LukeAbbyOP•2y ago

Ah nice, yeah I'd be happy to start work on it though to temper expectations for it, timeline for it would probably be measured in the weeks to months if I do it at all

Wasp•2y ago

Of course, no problem Sequencer is an unwieldy module, and popular too, so the longer something takes, the better 😄

LukeAbbyOP•2y ago

haha

Wasp•2y ago

Gives us time to hammer out bugs

LukeAbbyOP•2y ago

I'm thinking the easiest way would basically to add a permanent way to migrate the current effects to a timelined effect, that way they both can be treated the same under the hood.

Wasp•2y ago

I feel the actual canvas effect stuff needs a major rewrite, since that's the biggest and hardest thing to work with PIXI is hard, man

LukeAbbyOP•2y ago

Yeah I'm thinking moving it to work with the ticker would make sense since that way it lets you jump around the effect like delta is just a normal old variable that when on canvas gets updated by a ticker and you basically look up in the timeline what the state of all the transforms should be, like "opacity should be 0.64 and rotation should be 37.6 and..." it'll automatically be increased by the ticker but it'd also let you preview, pause, rewind, etc. an animation

Wasp•2y ago

Yeah, nice I think the hardest part will be the video though Like, the webm video

LukeAbbyOP•2y ago

I can see that, yeah

Wasp•2y ago

setting the currenttime is iffy at best, and the play and pause methods can throw errors if you're not real careful, or wrap everything in try and except :p

LukeAbbyOP•2y ago

Right, there's a few options but they're all tradeoffs The unusably bad option is to just load every frame of the video it's not actually time consuming to load the frames of the video into memory if you do it right it's memory consuming because even if you compress every image for every frame you don't get interframe compression and your memory usage skyrockets to a too unwieldy amount and if you try to add features like that back, congrats, you've remade the webm format but worse It's fine though we don't actually have to offer anything new at first for the timeline, so any video related problems already exist a MVP would just be something that can display the timeline for current effects then it'd be being able to edit the simpler stuff like fadeIn/fadeOut where it's just changing 500 to 750 or whatever then it'd be being able to edit much more complicated thing and require a new API for effects

Wasp•2y ago

Yeah, for sure Getting the timeline represent current behavior would be a great start With things like the animateProperty and such

LukeAbbyOP•2y ago

The dev level API could probably look SOMETHING like this, bear in mind I've written this in minutes so it's probably rough around the edges:

new Sequence()
    .timeline()
        .opacity(0) // Starts at opacity 0
            // `inKeyframe` means something that only runs when the effect is created, for something like `fadeIn`. `outKeyframe` would match `fadeOut`. These keyframes can "overlap" and interrupts normal keyframes.
            .inKeyframe(1, Sequencer.linear, 3) // Goes to 1 or 100% opacity linearly, taking 3 seconds
            // Keyframe order matters, it transitions FROM whatever the previous keyframe was (or in this case from the initial value, 0)
            .keyframe(0.75, Sequencer.linear, 1.3)
            // Transitions from 0.75 TO 0.95 exponentially over 1.7s
            .keyframe(0.9, Sequencer.exponential, 1.7)
        .tint("#000000")
            .keyframe("#FF0000", Sequencer.exponential, 2)
            .keyframe("#AA0000", Sequencer.linear, 0.32)
        .loop() // The keyframes are set up to all loop
        .attachTo(token)
    .start();

new Sequence()
    .timeline()
        .opacity(0) // Starts at opacity 0
            // `inKeyframe` means something that only runs when the effect is created, for something like `fadeIn`. `outKeyframe` would match `fadeOut`. These keyframes can "overlap" and interrupts normal keyframes.
            .inKeyframe(1, Sequencer.linear, 3) // Goes to 1 or 100% opacity linearly, taking 3 seconds
            // Keyframe order matters, it transitions FROM whatever the previous keyframe was (or in this case from the initial value, 0)
            .keyframe(0.75, Sequencer.linear, 1.3)
            // Transitions from 0.75 TO 0.95 exponentially over 1.7s
            .keyframe(0.9, Sequencer.exponential, 1.7)
        .tint("#000000")
            .keyframe("#FF0000", Sequencer.exponential, 2)
            .keyframe("#AA0000", Sequencer.linear, 0.32)
        .loop() // The keyframes are set up to all loop
        .attachTo(token)
    .start();

now... I don't know how important it is that you can write a script to do this actually. A Sequencer effect document could be added (??) and it'd be really easily editable in a timeline compared to like... editing in code. Even if there's not a document it'd be quite possible to export an effect in a JSON and you'd probably want to always edit the effect in the timeline, I feel like that's a LOT more convenient than editing by hand. I think a Sequencer effect document would have the benefit of being able to easily open on the sidebar and go to the timeline and edit, compared to a macro. Obviously you'd be able to refer to it in a macro but I just was feeling that as I began to write up the api that if you want to have even just dozens of keyframes (not unreasonable) it becomes really annoying. either way the idea is that it's a strict superset of new Sequence().effect() so internally a new Sequence().effect() would be converted to a timeline and then ran which'd simplify internal code, keeping from having to keep two versions around forever. I like this timeline js library the best so far... visually https://ievgennaida.github.io/animation-timeline-control/ We have a lot of time to figure out which looks best It needs more options though... for interpolation I like the visualisation better but this has the features we'd want: https://idflood.github.io/TweenTime/examples/basic.html I don't like how this one forces you to jump around while just editing keyframes but it's probably good enough to use too http://zz85.github.io/timeliner/test.html

Wasp•2y ago

yeah, these are pretty good examples

LukeAbbyOP•2y ago

I'd go with TweenTime in a second if it had a visualization a bit more like timeliner BTW heads up, Svelte is giving a bunch of A11y warnings @wasp heads up on the tip of the Svelte branch this errors:

new Sequence()
  .effect()
    .file("modules/jb2a_patreon/Library/Generic/Wind/WindStreams_01_White_20OPA_1200x1200.webm")
    .attachTo(token)
    .persist()
  .play();

new Sequence()
  .effect()
    .file("modules/jb2a_patreon/Library/Generic/Wind/WindStreams_01_White_20OPA_1200x1200.webm")
    .attachTo(token)
    .persist()
  .play();

Basically this.sprite only gets set up properly if it's jb2a database entry

LukeAbbyOP•2y ago

Okay so a potential reason for why Chrome sometimes lags until refresh seems to be because of uncoordinated effects. I can get Chrome to really not like when I use the same wind stream texture many times, even relatively coordinated. It mostly goes away upon refresh.

LukeAbbyOP•2y ago

the pixelation etc. is all visible on actual canvas, it's not an artifact of recording this is many tokens with the effect layered on top of each other but happens if you spread them out

Wasp•2y ago

Neat, how did you achieve this? I've fixed some bugs, so do make sure you have latest

LukeAbbyOP•2y ago

I think it's literally just reaching the limits of my graphics card It's uploading too much per frame for my GPU to handle because each effect has their own base texture I was trying to figure out why the performance gain of switching to one base texture was so high on firefox and it turns out it's a performance gain on Chrome too, you just have to be carefuller about setting up the effect I basically created the same wind stream effect on many many tokens. It seems if you create an effect too close in time to creating another it doesn't lag things, I think because Chrome keeps frames in memory better or something? I guess this raises a question to me about effect semantics this issue is going to mostly be with persisted effects if you have many persisted effects using a large enough texture you're going to have this problem it seems if you refresh the page they'll all get restarted at second 0 but when you're creating them one could be at second 5 and another at second 7 and yet another at second 11.6 etc. etc. So for performance reasons does it make sense to make all persisted effects that use the same underlying video run at the same exact frame?

Wasp•2y ago

That shouldn't be the case, there's a piece of code near the bottom in PersistentCanvasEffect that should calculate (roughly) where it should be in the loop based on its creation time But if that'll gain us some performance, then that's a good optimization

LukeAbbyOP•2y ago

Okay so it looks like it's a performance thing When the two videos are close enough they'll literally be tied together it seems, in Chrome, sometimes. the code does work to keep them offset on refresh I was just getting fooled by Chrome I had to set it up with some arrows to see it more clearly here

LukeAbbyOP•2y ago

Chrome

LukeAbbyOP•2y ago

Firefox

LukeAbbyOP•2y ago

I literally didn't change the effects. I didn't even re-set them up. I can consistently see this difference by just switching browsers, not changing the world at all. The idea here is that I set up arrows that fire at different timings because it's easy to visually tell when they're being tied together. In both Chrome and Firefox initially they're staggered (as I designed them to be). The difference is that Chrome on refresh/tab out/SOMETHING it eventually decides to tie the animations together for efficiency

LukeAbbyOP•2y ago

Here's another example of the "Chrome effect tying". You can see this time it's like "two volleys" this time On initial page load it just looks correct and identical to Firefox but if I tab out for a while behaviours like this start to happen

Wasp•2y ago

That's weird... Must be tabbing out paused them or something For resource saving reasons

LukeAbbyOP•2y ago

I'm nearly positive it's something like that and Firefox "does what you tell it to" hence why it doesn't cause the animation to get screwed up ...but at the cost of performance I think we really need to leave it up to devs, I guess temporary effects might as well always use their own video, I can think of less cases where it WOULDN'T be weird that temporary effects sync up, like arrows shouldn't sync up, fire bolts, etc. etc. but something like the inspire courage effect? That should sync up. This is actually the source of the particular performance bug my players were facing, there isn't any resource saving going on in Firefox. Inspire Courage (from PF2e Animations) uses a scaled down version of jb2a wind streams and when there were like 3 of them on screen one of my players with a low ends graphics card started to suffer. When there was like 6 it became borderline unplayable for several people using Firefox

Wasp•2y ago

Yeah, maybe we can add another option to .persist() that'll add a flag to reuse textures

LukeAbbyOP•2y ago

This is particularly unfortunate because Inspire Courage can effect everyone on the map at once quite possibly soooo yeah Yeah, something like that. Though it looks like Chrome is going to kill the fidelity of timing anyways for effects so maybeeee the whole idea of persisted effects having specific timings needs to be thrown out? I'd be happy starting with an opt-out to timing fidelity, it'd solve my specific problem That is genuinely a maybe but, like, if we tell Chrome to keep the timing fidelity I expect more people'll start experiencing performance problems but it clearly trashes timing pretty quickly And I don't have a bad graphics card

Wasp•2y ago

It is a possibility, but why wouldn't it lag from the get go if the timing offsets are there in that case?

LukeAbbyOP•2y ago

So it lags no matter what for my players on Firefox on refresh in Chrome it gets better or wdym because perhaps I was misleading earlier but like 3 Inspire Courages are enough to lag from the get go on Firefox

Wasp•2y ago

It does? Huh. Weird. Anyway, an option would be best regardless

LukeAbbyOP•2y ago

Yeah I think that's the outcome of my firefox performance stuff an option I initially hacked in ALWAYS using the same base texture for a video for all animations, this fixed the performance problems and since I didn't have any timing needs it didn't matter that I broke it I then wrote a much more involved version that tried to only reuse a base texture if it's within the same timing however even this version has bugs because of creation date offset and I'm starting to realise because of creation date offset it'd basically go back to one base texture per effect because the number of effects on screen that have EXACTLY the same creation date are going to be basically 0. I could bucket it if the creation date is close enough... but you catch the drift it's still a basically unworkable solution because if the creation date, the start time, or the end time are different then there has to be a different base texture so TLDR; yeah, devs need to opt into this for their effect I feel like I learned a LOT researching this lol, though I'm going to have to throw away all the code I wrote I think I'm going to start documenting a "Performance" section or something. There's a lot of intricacies here.

Wasp•2y ago

Is the texture-sharing something you'd like to implement, btw?

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

Yeah. I'll PR it soon

It'll probably be a bit, the PR itself shouldn't be that large but so far I've spent a lot of time researching efficacy more than actually writing what'll end up in final code and I won't get to it again most likely until weekend I do need to look further into this because asfik these videos should have playNaturally flagged etc. so all setTimeouts should be avoided based upon my initial reading of the code but I realised that that's a counter-hypothesis, that setTimeouts are the only timing that Chrome is reducing the fidelity of. Yeah I'm disappointed by the state of video APIs in browsers. Firefox seems to be a bit behind Chrome but... overall no browser seems to have the kind of performance-centric features that'd be really nice

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

The main problem I think that has is the absolutely horrendous memory usage Like if you have a 100MB video loading each frame for an animated sprite will bring that to an absolutely unwieldy memory figure, right? (even a 10MB video gets really large if you load each frame, I think I was hearing 80x increase but idk if that figure is the norm) obviously character animations or whatever should be but that's usually dealing with no more than a few dozen frames

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

yeah I don't really think the performance characteristics of offloading to a worker are much better at least for me the CPU utilisation is not the limiting factor

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

Yeah that's what I was seeing syncVideo is the option that'd basically turn off video texture cloning but it'd have to be conservative and currently as designed is opt-in

Unknown User•2y ago

Message Not Public

LukeAbbyOP•2y ago

Chrome seems like it might resync them or something based upon what I was seeing but yeah, syncVideo would be the thing that'd allow you to ask for them to be synced (aka same base texture) honestly I already know how to implement it I in fact have already implemented a version of it that's still subtly broken it's... annoyingly hacky though because basically you need to know the video's length to calculate the duration of the effect... but you need to know the duration of the effect to calculate whether you can cache the video or not. The reason for that is basically startTimePerc/endTimePerc If endTimeDuration is 25% you need to know the video duration to know the concrete second that the video ends at I do think I came up with a solution it's just kinda spaghetti If core were to remove start time and end time and offsets I'd basically recommend the same thing but I think that's probably an unacceptable change you couldn't create that arrow volley effect example, I've legitimately seen people doing an effect like that I think there's a lot of interesting, frankly JIT-y optimisations that might help But each will require a LOT of research because choosing the wrong breakpoints would hurt performance more than it helps Another method that can help performance; alternative asset sizes primarily for like scaleToObject. I don't know how many assets in the JB2a library this would currently be applicable so maybe it's not that helpful for existing effects? But here's the idea:

new Sequence()
  .effect()
    .file({
      "125x125": "jb2a.wind_stream.125x125.white", // Declare asset sizes. I'm using a ficticious database path as there's currently not sizes of wind stream besides 1200x1200. The API is based upon the distance-based effect selection. If the scaling is rectangularthe longer side length is used.
      "300x300": "jb2a.wind_stream.300x300.white",
      "410x410": "jb2a.wind_stream.410x410.white",
      "1200x1200": "jb2a.wind_stream.white",
    })
    .scaleToObject(x); // When scaled will automatically choose the closest asset for the scaled to size.

new Sequence()
  .effect()
    .file({
      "125x125": "jb2a.wind_stream.125x125.white", // Declare asset sizes. I'm using a ficticious database path as there's currently not sizes of wind stream besides 1200x1200. The API is based upon the distance-based effect selection. If the scaling is rectangularthe longer side length is used.
      "300x300": "jb2a.wind_stream.300x300.white",
      "410x410": "jb2a.wind_stream.410x410.white",
      "1200x1200": "jb2a.wind_stream.white",
    })
    .scaleToObject(x); // When scaled will automatically choose the closest asset for the scaled to size.

This by itself can immediately decreases GPU load by uploading a lower resolution from the get-go if possible. This doesn't defeat syncVideo or anything though. If the asset in mind is an image or if they're a video with syncVideo set to true you can have them be backed by one base texture. Say if there's 5 effects on screen and the largest is 300px, they can ALL use the 300px base texture scaled down for their needs. I'm pretty sure this'll always be more performant because scaling down is cheap once the resource has been uploaded (asfik). I guess that assumes all sizes are identical so maybe a flag to surface whether that's safe to do would be good. Like if the effect changes based upon its size... like say a tiling effect or something (though that case should probably be handled separately). BTW now that I think about it, I think syncVideo should just be an effect option, not just for persisted effects option. If you make a couple of temporary effects to fire a volley of 10 arrows and you don't mind them being coordinated there's going to be measurable performance gain from using syncVideo there. (hopefully this asset size choosing doesn't exist already, I poked around looking for it but couldn't find anything) one of the things that sucks for Foundry specifically is so many of the more advanced APIs REQUIRE an https connection... And since it's self-hosted you really can't count on that

LukeAbbyOP•2y ago

There's honestly too much to look at in terms of optimisation. I'm starting with those two things because I know they have concrete performance gains but like I come across so much like this: https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/WebGL_best_practices#consider_compressed_texture_formats This is a cool thing I had a hint of earlier but I have NO idea how much of a performance gain it'd be. I don't think I'll implement it at all even.

WebGL best practices - Web APIs | MDN

WebGL is a complicated API, and it's often not obvious what the recommended ways to use it are. This page tackles recommendations across the spectrum of expertise, and not only highlights dos and don'ts, but also details why. You can rely on this document to guide your choice of approach, and ensure you're on the right track no matter what brows...

Wasp•2y ago

Nice idea - I would definitely want to push this to be a database driven thing however, so that end users won't have to fiddle with this

LukeAbbyOP•2y ago

do you know how many assets have multiple sizes already?

Wasp•2y ago

I do not, I think that's best asked towards the JB2A guys I know bless has two One thing that the Sequencer database does in 3.0.0 is that if it runs into any file key in the object it is recursing through, it will treat that like a SequencerFile. This way you can embed metadata through the _ prefix, and through the file key, to perhaps achieve something like this:

{
  "bless": {
    _scalable: true,
    file: {
      "400x400": "path",
      "200x200": "path
    }
  }  
}

{
  "bless": {
    _scalable: true,
    file: {
      "400x400": "path",
      "200x200": "path
    }
  }  
}

bless will then become its own distinct database entry with the metadata of scaleable: true, so you can make decisions how to handle it

LukeAbbyOP•2y ago

hmm, do you think _scalable should just be automatically detected based upon the type of file? There's at least: - Bite (200px, 400px) - Bless (200px, 400px) - Claws (200px, 400px) - Cure Wounds (200px, 400px) - Dizzy Stars (200px, 400px) - Fire Ring (500px, 900px) - Flaming Sphere (200px, 400px) - Healing Generic (200px, 400px) - Sphere of Annihilation (200px, 600px)

Wasp•2y ago

The metadata is there regardless the file key object would just allow for that programmatic behavior as well

LukeAbbyOP•2y ago

Ahhh okay, well I'll take a look at it eventually. I don't know much about the database end but the feature itself shouldn't be that hard judging upon what I know right now

Wasp•2y ago

3.0.0 is almost done I feel - do you want to aim for your improvements in 3.0.x?

LukeAbbyOP•2y ago

Depends, I might do it this weekend Is "almost done" like a few weeks from now or a few days

Wasp•2y ago

This has been implemented in the latest version with good success, thanks for the feedback

Leo The League Lion•2y ago

@wasp gave LeaguePoints™ to @EBER (#39 • 71)

Wasp•2y ago

I can wait until monday I've got Monday off from work, and since it's not the weekend, I feel I have the week to fix any bugs in time for weekend games

LukeAbbyOP•2y ago

gotcha, yeah I just didn't work on it at all these weekdays since I have my own job I'll get the PR in this weekend then

Wasp•2y ago

No pressure dude, I'm happy to roll it in a new update when you get it done The last thing I want is to make it another job for ya 😄

LukeAbbyOP•2y ago

it's probably about when I'd have wanted to finish it anyways Which branch? Svelte was where I was putting things but I want to make sure it's right still

Wasp•2y ago

Svelte yeah It's a CHONKY changelog - Sequencer - Updated Sequencer Database Viewer: - Improved UI and added nested tree view - Added ctrl modifier to buttons that copy paths, which adds quotes around the copied paths - Sequencer - Updated Sequencer Effect Player: - Improved UI based on the design of MatthijsKok on github - thanks a lot for the inspiration! - Sequencer - Reworked the Sequencer Effect Manager to the Sequencer Manager: - Added the ability to stop running sounds - Added a Sequence view where you can see the sequences as they are running, and stop the entire execution or their individual sections - Sequencer - Added .scrollingText() which allows playing scrolling text on the canvas for users - Sequencer - Added .canvasPan() which allows panning the canvas for connected users - Sequencer - Added .toJSON() and .fromJSON() to Sequences to be able to be serialized and deserialized; only sequences with effects, sounds, scrolling texts, and canvas pans can be serialized - Sequencer - Added options to .play(), which may contain an object; currently supports { remote: true/false } which will serialize the sequence (see above), and send it to each client for local playback, instead of the person running the sequence sending data to clients as it is being executed - Sequencer - Added database support for _timestamps metadata on effect files, which will trigger the sequencerEffectTimestamp hook when effects reach the point of the timestamps for that file - Sequencer - Added support for flipbook-type effects through a _flipbook database tag - Animations - Improved playback of movement, fade in/out, and rotation animations on tokens - Effects - Added CanvasEffect#addAnimatedProperties, which will allow you to easily add animations to properties of existing effects - Effects - Improved screenspace above UI effect performance by not rendering the extra canvas when not in use - Effects - Fixed screenspace effects being affected by the vision mask - Effects - Fixed .stretchTo() effects would be visible when not in vision - Effects - Fixed .fadeOut() and .scaleOut() not working at all - Effects - Reworked how effects are replicated on linked tokens when .persist()'s persistPrototypeToken is enabled, improving performance

LukeAbbyOP•2y ago

CHONKY Yeah my PR would have had the URL.createObjectURL change in it

Wasp•2y ago

ah, cool yeah that's how I did it:

this._videos[inSrc] = {
  objectURL: URL.createObjectURL(blob),
  size: blob.size,
  lastUsed: +new Date(),
};

this._videos[inSrc] = {
  objectURL: URL.createObjectURL(blob),
  size: blob.size,
  lastUsed: +new Date(),
};

async function get_video_texture(objectURL) {
  return new Promise(async (resolve) => {
    const video = document.createElement("video");
    video.preload = "auto";
    video.crossOrigin = "anonymous";
    video.controls = true;
    video.autoplay = false;
    video.autoload = true;
    video.muted = true;
    video.src = objectURL;

async function get_video_texture(objectURL) {
  return new Promise(async (resolve) => {
    const video = document.createElement("video");
    video.preload = "auto";
    video.crossOrigin = "anonymous";
    video.controls = true;
    video.autoplay = false;
    video.autoload = true;
    video.muted = true;
    video.src = objectURL;

LukeAbbyOP•2y ago

Yeah, pretty simple honestly you do need to revoke it and all etc.

Wasp•2y ago

I revoke it if the cache gets too big

LukeAbbyOP•2y ago

Right but if the video errors

Wasp•2y ago

Ah yeah no I'll revert my changes on the file cache then

LukeAbbyOP•2y ago

Well I mean do you even want to bother retrying to get the blob if it errors? it's technically a change, right?

Wasp•2y ago

100% i was more referring to the overall url changes in that file I'll let yer PR handle it

LukeAbbyOP•2y ago

Ah, well, I wouldn't want to not implement something you did but yeah I'll look at your diff IDK your release cadence but maybe timeline stuff could be mature enough for 4.0.0 I've been toying around with the idea for a while and I think I'm committed to at least making a read-only timeline Anyways that's not what I'll PR this weekend I bring it up because I did have a question about what you think of syncVideo, in that if we move to timelining things it'd probably be quite possible to say "play this video for 5 seconds, then play this video for 10 seconds" etc. etc. and the meaning of syncVideo gets trickier at that point. I think syncVideo will get into 3.0.0 but scalable probably won't. (The asset size changing thing) because scalable is a lot more niche

Wasp•2y ago

Yeah, for sure Do you think we could exclude .md files from the commit linting workflow? It messes with the sequencer-esque linting 😄 Ie,

new Sequence()
  .effect()
    .atLocation(token)
    .file(
      "modules/jb2a_patreon/Library/Generic/Healing/HealingAbility_01_Blue_200x200.webm"
    )
    .fadeIn(500)
    .fadeOut(500)
  .play();

new Sequence()
  .effect()
    .atLocation(token)
    .file(
      "modules/jb2a_patreon/Library/Generic/Healing/HealingAbility_01_Blue_200x200.webm"
    )
    .fadeIn(500)
    .fadeOut(500)
  .play();

becomes

new Sequence()
  .effect()
  .atLocation(token)
  .file(
    "modules/jb2a_patreon/Library/Generic/Healing/HealingAbility_01_Blue_200x200.webm"
  )
  .fadeIn(500)
  .fadeOut(500)
  .play();

new Sequence()
  .effect()
  .atLocation(token)
  .file(
    "modules/jb2a_patreon/Library/Generic/Healing/HealingAbility_01_Blue_200x200.webm"
  )
  .fadeIn(500)
  .fadeOut(500)
  .play();

LukeAbbyOP•2y ago

Ah oops, absolutely In the module.json:

  "lint-staged": {
    "*.{js,css,md}": "prettier --write"
  }

  "lint-staged": {
    "*.{js,css,md}": "prettier --write"
  }

change it to:

  "lint-staged": {
    "*.{js,css}": "prettier --write"
  }

  "lint-staged": {
    "*.{js,css}": "prettier --write"
  }

then add a .prettierignore file and add a line with **/*.md I can make a PR for this if you'd like

Wasp•2y ago

Plz do 😄

LukeAbbyOP•2y ago

I love git magic I'm going to be able to revert all the md files in one command, back to pre-prettier

LukeAbbyOP•2y ago

https://github.com/fantasycalendar/FoundryVTT-Sequencer/pull/159

GitHub

Fix markdown by LukeAbby · Pull Request #159 · fantasycalendar/Foun...

Markdown files when formatted were killing some of the intentional spacing in new Sequencer() examples. This makes sure md files won't be formatted in the future and reverts the formatting that...

LukeAbbyOP•2y ago

I spent a bit of time walking through the changes but this is basically the opposite of what I did in #156 to md files wait did you push something to the Svelte branch like, really recently Cuz I pulled Svelte, then made fixMarkdown and then made the PR and then it looks like Svelte got updated in between those two steps because I had to pull Svelte again and merge it into fixMarkdown. Anyways it should be done now.

Wasp•2y ago

Thaaaaanks! 😄 I did yeah I've been adding a lot of shiz Mainly user facing stuff

LukeAbbyOP•2y ago

right, like last 30 minutes though?

Wasp•2y ago

Yeah

LukeAbbyOP•2y ago

haha okay yeah that explains it I hope Prettier has been pleasant so far (?) I really don't want it to feel like I'm imposing it upon your repository haha I just thought you should either apply prettier or remove the prettierrc file I personally found it convenient to not have to care about formatting after a while

Wasp•2y ago

It's amazing I prefer it 100% over running it locally and forgetting 😛

LukeAbbyOP•2y ago

haha, I have my IDE set up to do it on all file changes what IDE do you use? BTW is there a more permanent place you'd like to move this? I don't mind just continuing to use this thread but I figure there might be a good place already.

Wasp•2y ago

I use PHPStorm because I develop Fantasy Calendar when I'm not poking around in Foundry 😄 Here is fine, alternatively you could join the Fantasy Computerworks discord server, but I appreciate dev7355608's input, when they do drop by 😄

LukeAbbyOP•2y ago

https://www.jetbrains.com/help/phpstorm/prettier.html

PhpStorm Help

Prettier | PhpStorm

LukeAbbyOP•2y ago

Okay I've discovered something cool The whole video seems to be loaded into memory, at the least for small videos.

      console.log(video.duration);
      console.log(
        video.seekable.start(0), // 0
        video.seekable.end(0), // video.duration
        video.seekable.length // 1
      );

      console.log(video.duration);
      console.log(
        video.seekable.start(0), // 0
        video.seekable.end(0), // video.duration
        video.seekable.length // 1
      );

seekable is analagous to a [](start, end) by that I mean it looks like this:

[
  [0, 1], // 0 seconds to 1 seconds is buffered
  [1.7, 2], // 1.7 seconds to 2 seconds is buffered
]

[
  [0, 1], // 0 seconds to 1 seconds is buffered
  [1.7, 2], // 1.7 seconds to 2 seconds is buffered
]

Except since it's read only you access it with start(i) and end(i), in most cases I'm seeing that the whole video is loaded this means we can maybe get one video element since if it's saying it's all buffered seeking around could be cheap HOWEVER I need to do more research whether that'll be more performant in all cases or if not what the heuristic should be. Plus syncVideo will still be necessary to reduce load on the GPU's end

Wasp•2y ago

I'm curious to see how you could use a single base texture for this Would you have to generate a texture from it?

LukeAbbyOP•2y ago

Yeah, seeking through the video a bunch. It won't solve the problem because you're still uploading too many frames of the video the GPU every tick, it'd only limit the number of video elements which might be beneficial Also @wasp sorry to say but I think syncVideo will have to wait for, say, 3.1.0 as I've discovered new edge cases. The main one I hadn't stumbled into yet is video audio fade in I think canvas-effects.js needs a big refactor at some point. It's really complicated, it makes sense because of all that it's doing but adding this new feature has been difficult because I'm basically rewriting all the video logic and it's >3.5k lines long so trying to make sure I don't break anything got difficult. There's a lot of "effect at a distance" which is why I didn't discover some of these things at first I also wanted to ask, are methods like playMedia and pauseMedia etc. meant to be able to be called by modules that use Sequencer? Basically do I have to assume stuff like playMedia and pauseMedia can be called at ANY time from any source? To clarify this point, since all the videos are to be synced up, modifying the underlying video will simultaneously effect all synced videos. You could technically work around this by adding n sound tracks but 1 video but that'd sound worse especially since sound should sync with the video so... The problem comes in that if you, say, add an effect while the audio is 10s in you don't really want to apply a fadeIn since you're not at the beginning of the effect where the fadeIn is meant to apply. Say you're fading in an electric buzz or something, you don't want adding a second electric buzz to apply fade in as well, I think. Jumping all the effects back to the beginning is similarly unideal. I think the answer is to apply audio fade in only for the first video and fade out only for the last video but I don't think I'll manage to implement everything today. I don't know if this intersection of features will even be used, a synced video with audio set to fade in seems a bit niche, but... well I have to code it in.

Wasp•2y ago

They are to unify the behavior between animated sprites and sprites with videos instead of having to litter the class with ifs and elses for various types of media, I have separated out those into a single function that can handle the edge cases for that

LukeAbbyOP•2y ago

Right but are they considered a part of the public API?

Wasp•2y ago

Nope, they are not

LukeAbbyOP•2y ago

I noticed they're not private methods Okay good That simplifies things Not enough to get the feature fully working today But enough to not have to support some fairly complicated video splitting basically It's funny the basis of the feature took minutes That is finding where the base texture was being created and making it cached All the cases where that breaks things have been the time consuming part

Wasp•2y ago

Yup. Sequencer itself is straightforward. The canvas effects... are not. alrighty then, 3.0.0 release tomorrow it is

LukeAbbyOP•2y ago

Yeah that was the plan either way, I believe, but syncVideo just can't make it in unfortunately I basically am rewriting how videos work entirely in canvas effects just to let you know the scope it's, apparently necessarily, crept into It should actually simplify canvas-effects.js

Wasp•2y ago

Neat, can't wait to see what you're cooking up

LukeAbbyOP•2y ago

I don't think it'll be an amazing difference but hopefully it'll be an improvement. I have to do it because the code mutates the video a lot (to set up looping over a time range etc.) and that's all a no-go if it's synchronized as that'd end up changing all the synchronized canvas effects and cause jumping etc. ...wait another edge case. How should no loop interact with this?

Wasp•2y ago

No loop just plays the video until the end, and stops, I believe very rarely used if the set duration of the sprite is greater than the duration of the video, it should remain on the last frame

LukeAbbyOP•2y ago

I think that could be surprising if you're synchronized because if the video you're synchronized to is currently 4.5s in and it's a 5s video well you'll be done with the video in 0.5s and linger on the last frame for 4.5s But I suppose that's one of the only sane approaches, the other making it an error probably

Wasp•2y ago

yep, entirely possible to add in the validation step of .effect() it complains about .scaleToObject() combined with .stretchTo() for example

LukeAbbyOP•2y ago

Yeah I think that makes some sense. Honestly I'm tempted to add a "effect group" where it can contain a bunch of sub-effects. All effects inside it would start at the same time and such and their positions relative to the origin could be saved. It'd be nice to be able to place down 10 lights in a circle at a time or something. It seems nice in general but I was thinking about it because it'd solve the case of noLoop + syncVideo, since if you can tell that multiple videos are intended to start at the same time then you can just sync them, aka if they're in the same effect group. Even if you just run an animation of a light going out. Semantically you'd basically put every effect into a global effect group, the problem with having synchronised effects work globally is just that even if effects are created milliseconds apart it's hard to tell if they're meant to sync apart. Granted it'd be easy to say if they're within a small duration they sync but that doesn't fix the case of adding multiple effects slowly, like inspire courage. This of course is a lot more work than syncVideo and that basically needs to be implemented anyways if the optimising part of "effect group" were to be done, I'll be continuing implementing that. One interesting idea of an "effect group" could be "baking" the animation. I'm stealing the term from 3d modelling but the basic idea is instead of having like 10 videos in an effect group, you can pre-export it into one video. It would potentially allow running more ambitious effects without dropping performance that badly. I think it might be just as slow though and potentially slow down things if not done carefully. This is basically trying to replace a video editor at that point though, I think the easiest way to implement that would be to tell people to just do that outside of Sequencer. But it's a cool idea. Basically syncVideo + timeline are my two 'good' ideas so to say, things I'm willing to try to implement, the rest is mostly ponderings.

Wasp•2y ago

It has been released.

LukeAbbyOP•2y ago

🎊 Congrats And thanks

LukeAbbyOP•2y ago

So I haven't been working on this for the past while because, well I got busy with work but I wanted to post an update on timeline stuff:

LukeAbbyOP•2y ago

Ignore localization not being reloaded for some reason. This is TECHNICALLY a working import of an effect into a timeline. The lighter ones are the colors for "transition in" and the darker blue you can see is "transition out" the issue I've run into is that, well you can actually see, all the "transition" out are squished near one second because it seems like the effect's duration is calculated as 990 ms This is the effect in question, I'm just trying to use as many things as possible:

new Sequence()
    .effect()
        .delay(1000)
        .atLocation(token)
        .file("jb2a.fire_bolt.orange")
        .fadeIn(100)
        .fadeInAudio(500)
        .fadeOut(1000)
        .fadeOutAudio(1000)
        .scaleIn(0.5, 250, { delay: 1000 })
        .scaleOut(0.5, 500, {ease: "easeOutCubic", delay: -100})
        .volume(1.1)
        .stretchTo(game.user.targets.first())
        .timeRange(10, 1000)
        .persist()
    .play();

new Sequence()
    .effect()
        .delay(1000)
        .atLocation(token)
        .file("jb2a.fire_bolt.orange")
        .fadeIn(100)
        .fadeInAudio(500)
        .fadeOut(1000)
        .fadeOutAudio(1000)
        .scaleIn(0.5, 250, { delay: 1000 })
        .scaleOut(0.5, 500, {ease: "easeOutCubic", delay: -100})
        .volume(1.1)
        .stretchTo(game.user.targets.first())
        .timeRange(10, 1000)
        .persist()
    .play();

I understand why it's calculating 990ms, it's looking at the time range of 10ms to 1000ms so a duration of 990ms but like, as you can see the scaleIn alone has a delay of 1000ms and a duration of 250ms so I feel like it should be 1250ms. Is that correct and the existing duration calculations don't take that into account or am I calculating things wrong. @wasp Ping for visibility since the thread is old at this point. The current goal of this I'd like to try to PR a read-only version. I'll probably leave it there short term because frankly long term this requires a whole refactor of canvas effects which as you're well aware is a beast. I think it'd be worth it but it's tricky Writable version doesn't necessarily have to be long-term the main problem is that it's... weird? My current idea for the approach is to have it so that it reads and writes from like a macro. Really it feels like it should be an 'item' but that's not system agnostic so macro is probably what it'd have to be. Though maybe they shouldn't be macros so they're hidden from macro listings. I'm not sure what strategy is best. Regardless it'd probably be a good idea to add a "Sequencer Effect Browser" to the buttons to browse these keyframed effects. I think this in its entirety would basically deprecate the Sequencer Manager Player, at least eventually.

Wasp•2y ago

This looks cool, as for your question, I think that may be an oversight, the largest duration should be picked, and the time range should just loop between 10 and 1000 in this case.

LukeAbbyOP•2y ago

So delay should count for duration? Even for transition in/out?

Wasp•2y ago

Maybe, what do you think? It might become confusing, no?

LukeAbbyOP•2y ago

Well I could see it being a bit confusing but delay in general could be confusing Like fade out how does a delay even work? Because even a small delay means you cut off part of the fade out right? At least I'm pretty sure it has to if there's no loop or anything that is Then negative delay at fade out, how does that work for persisted effects? I think the answer is that negative delay just doesn't do anything for persisted effects, it's just like delay 0 The main reason I ask at all is basically I want to figure out how the design is currently intended to work because I can try to be feature complete to what's currently happening but not to what's actually desired.

Wasp•2y ago

I feel that only the duration of the in/out methods should contribute to the overall duration, eg, if you have a duration of 250, and a fadeOut of 500, the duration is 500. The delay is merely an offset for when the animation starts

LukeAbbyOP•2y ago

Fair enough Okay so if there's like a fade out and a fade in that could run at the same time which should take priority? Say you set it to fade in for 1 second with a 1 second delay and fade out for 1 second and the effect is 2 seconds long

Wasp•2y ago

I think outs would take precedence, but that's not currently the case We can't account for every user error lol

LukeAbbyOP•2y ago

I know I'm just turning these effects into timelines and I have to consider these edge cases for how I convert the effects

LukeAbbyOP•17mo ago

I am still working on this, just sporadicly on the weekend

LukeAbbyOP•17mo ago

The UI of the timeline is basically done, albiet that's probably one of the easier parts because I'm just using a library I basically need to rewrite the effects from scratch now lol @wasp how do you feel about Typescript? I'm not going to suggest anything like rewriting the whole code base in Typescript but I'm wondering if you'd be for or against writing some components in Typescript, Svelte makes it very easy to do that and I'm mildly missing types occasionally. if you don't like Typescript, don't know Typescript, etc. no problem, I haven't written any components in Typescript so far or anything but I'm considering doing it

Wasp•17mo ago

I'm not against it, but it does get harder to get others to contribute to the module if it's used

Gaming

Programming

Sequencer Firefox Performance fixes