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.

Sign up for GitHub

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

Jump to bottom

setTransform turns into getTransform on Chrome #97

Closed
DennisBengs opened this issue Mar 30, 2019 · 8 comments
Closed

setTransform turns into getTransform on Chrome #97

DennisBengs opened this issue Mar 30, 2019 · 8 comments
Assignees
@Siorki
Labels
bug
Milestone
5.0.4

Comments

@DennisBengs
Copy link

DennisBengs commented Mar 30, 2019 •
edited
Loading

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
Copy link
Owner

Siorki commented Mar 31, 2019

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

  • run RegPack once, take the output from the "Preprocessing" block, and manually edit it to revert the instances c[c.er] into c.getTransform or c.setTransform
  • copy the edited block back into the input field of RegPack, disable the "method hashing and renaming" option on the first line, and run it again.

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.

@Siorki Siorki self-assigned this Mar 31, 2019
@Siorki Siorki added the bug label Mar 31, 2019
@DennisBengs
Copy link
Author

DennisBengs commented Mar 31, 2019 •
edited
Loading

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
Copy link
Owner

Siorki commented Mar 31, 2019

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 :

  • Firefox 66 : hash i[1]+[i[4]] and c.setTransform() becomes c[c.er]
  • Chrome 72 : hash i[0]+i[4] and c.setTransform() becomes c[c.sr]
    (I surmise that if I upgrade Chrome to 73, there is no way back)

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
Copy link
Author

Yepp, both are listed:

"getTransform [er]",
"setTransform [er]",

@Siorki
Copy link
Owner

Siorki commented Apr 1, 2019

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
Copy link
Owner

Siorki commented Apr 3, 2019 •
edited
Loading

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
Copy link
Author

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
Copy link
Owner

Siorki commented Apr 24, 2019

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.

@Siorki Siorki added this to the 6.0 milestone Apr 24, 2019
@Siorki Siorki modified the milestones: 6.0, 5.0.4 Jun 29, 2022
@Siorki Siorki closed this as completed in d08930d Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Siorki Siorki

Labels
bug
Projects
None yet
Milestone
5.0.4
Development

No branches or pull requests
2 participants
@Siorki @DennisBengs