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

fix clipped cache when using multiple objects with cacheless filters #979

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adabru
Copy link

@adabru adabru commented Aug 23, 2018

Following code will clip objectBig to the size of objectSmall on the second update call:

var stage = new createjs.StageGL(, {directDraw:false})
objectSmall.filters = []
objectBig.filters = []
stage.addChild(objectSmall, objectBig)
// first update call
stage.update()
// second update call
stage.update()

https://codepen.io/adabru/pen/aaOxZb?editors=1000

The error comes from following code:

for (var i = 0; i < l; i++) {
var item = container.children[i];

if(!this._directDraw && (!ignoreCache && item.cacheCanvas === null && item.filters !== null && item.filters.length)) {
var bounds;
if (item.bitmapCache === null) {
bounds = item.getBounds();
item.bitmapCache = new createjs.BitmapCache();
item.bitmapCache._autoGenerated = true;
}
if (item.bitmapCache._autoGenerated) {
this.batchReason = "cachelessFilterInterupt";
this._renderBatch(); // <----------------------------------------------------
item.alpha = 1;
var shaderBackup = this._activeShader;
bounds = bounds || item.getBounds();

On the first update call, every item will go into the if (item.bitmapCache === null) branch and the bounds variable will be assigned to the correct bounds. On the second update call however, no item will go into this branch as bitmapCache is assigned for each of them. Therefore bounds is assigned by the line bounds = bounds || item.getBounds();. As bounds is null on declaration, the first item will get its correct bounds assigned with this statement. BUT as bounds is declared as var and therefore its scope is for the entire function, the next items will pick the first item's bounds and thus be clipped if they are smaller than the first item.

One fix could be to use var bounds = null;. The fix in this pull request is using let bounds; instead to block-scope the bounds variable.

See https://codepen.io/adabru/pen/rZVZyX?editors=1000 .

let syntax is not handled by grunt-contrib-uglify default branch, so this pull request also changes this dependency to its grunt-contrib-uglify-es branch (https://github.com/gruntjs/grunt-contrib-uglify/tree/harmony).

My workaround at the moment is using a fork of this repo. I hope this issue can soon be fixed in master.

@adabru
Copy link
Author

adabru commented Aug 24, 2018

Ok, I have a new workaround by just patching this function with overriding createjs.StageGL.prototype._appendToBatch. So I have no inconvenience with this issue anymore and priority faded away.

@DavidHGillen
Copy link

Thanks for the detailed writeup of this. I want to take some time and study the problem, make sure it's not indicative of something else getting missed. We'll let you know when we merge a fix for this even if it's not the PR. Glad you're able to work around it though!

@lannymcnie
Copy link
Member

We are looking this week at merging this.

  1. Can you adjust the pull request to not commit the compiled files?
  2. The let/var change is also not ideal, since the 1.x branch can still be used without any transpiling/minifying. EaselJS still works with Safari 9 and other browsers that don't yet support let.

@adabru
Copy link
Author

adabru commented Feb 4, 2019

Done. The only and sufficient change is that the bounds are reassigned to null after each iteration so that no child is evaluated with the bounds of its previous sibling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants