Siorki / RegPack

Self-contained packer for size-constrained JS code
Other
301 stars 14 forks source link

setTransform turns into getTransform on Chrome #97

Closed DennisBengs closed 2 years ago

DennisBengs commented 5 years ago

Straightforward problem. When it should use setTransform in Chrome 73.0.3683.86 it uses getTransform instead.

Hash properties: for(i in c)c[i[1]+[i[4]]]=i

getTransform/setTransform c[c.er](...)

Siorki commented 5 years ago

First, if you need a quick workaround, since js1k deadline is so close :

Now let's focus on the issue itself. i[1]+[i[4]] is quite an unusual hash. Can you please copy the contents of the information box on the right side of the preprocessed code ? This will list the hashed methods and help me reproduce the issue. Thanks.

DennisBengs commented 5 years ago

Oh I see. I thought the hash was hard-coded based upon two letters which were decided to represent all properties uniquely.

I already fixed the issue in my own code by replacing setTransform with save/restore, but here is a minimal example which will reproduce the problem:

c.setTransform();
c.bezierCurveTo();
c.bezierCurveTo();
c.fill();

----------- Hashing methods for 2D context -----------
Renamed methods for c
c.er -> setTransform
c.ee -> bezierCurveTo
c.i -> fill

Loop variables :
No relevant loop found, defaulting to i

Strings present in the code :

Wrapping packed code in :
 ' : cost = 0
 " : cost = 0
 ` : cost = 0

----------- Renaming variables to optimize RegExp blocks --------
All variables : ci
in keywords only : efnor
in variables only : (none)

No variables to rename.
Siorki commented 5 years ago

The hash is not static, RegPack has a list of possible hashes and tries to find the best match I was able to replicate the issue with a different configuration :

Under Firefox, getTransform() is no longer listed in the methods of a 2d context. Can you please copy the following code to a html file, open it with Chrome 73, and let me know if you have both getTransform and setTransform listed ?

<html><head>
</head>
<body>
  <div id="d"></div>
</body>
<script>
var a = document.createElement("canvas");
var c = a.getContext("2d");
var methods = [];
for (var i in c) {
    methods.push(i + " ["+i[1]+[i[4]]+"]");
}
methods.sort();
var text = methods.join('",<br>"');
document.getElementById("d").innerHTML=text;
</script>
</html>
DennisBengs commented 5 years ago

Yepp, both are listed:

"getTransform [er]",
"setTransform [er]",
Siorki commented 5 years ago

getTransform() actually seems to be part of the WHATWG HTML Living Standard However, it is not listed in the W3C 2D Context Recommendation

I'll upgrade Chrome tomorrow and see how it behaves.

Siorki commented 5 years ago

Switched to Chrome 73.0.3683.86 I get the same behavior as in 72 : both getTransform() and setTransform() are known, and the hash for the sample is i[0]+i[4].

Did you pack your code with Firefox, then attempt to run it with Chrome ?

DennisBengs commented 5 years ago

Did you pack your code with Firefox, then attempt to run it with Chrome ?

Yes I did. Honestly didn't occur to me to try regpack in Chrome.

Siorki commented 5 years ago

So this is issue #20 , reloaded with recent versions of the browsers. The list of methods differ between Chrome and Firefox and the solution is the same as back then : update ContextDescriptor to even out the differences.