Skip to content

Conversation

@eoghanmurray
Copy link
Contributor

…when there are multiple being patched - all were referring to the same one meaning that variables weren't being indexed correctly as canvasVarMap in serialize-args.ts was only seeing a single context

If you revert the change from prototype to this from this commit, you'll get the following failure in the new test added in this commit:

@@ -199,11 +199,11 @@
"property": "bindBuffer",
"args": [
34962,
{
"rr_type": "WebGLBuffer",
- "index": 0
+ "index": 1
}
]
},

This is because the 'confound' canvas was populating a single entry in the canvasVarMap so when 'myCanvas' starts populating, there is already an entry (which doesn't exist on the replay side as the 'confound' canvas was never emitted)

…when there are multiple being patched - all were referring to the same one meaning that variables weren't being indexed correctly as `canvasVarMap` in serialize-args.ts was only seeing a single context

If you revert the change from `prototype` to `this` from this commit, you'll get the following failure in the new test added in this commit:

   @@ -199,11 +199,11 @@
                "property": "bindBuffer",
                "args": [
                  34962,
                  {
                    "rr_type": "WebGLBuffer",
    -               "index": 0
    +               "index": 1
                  }
                ]
              },

This is because the 'confound' canvas was populating a single entry in the canvasVarMap so when 'myCanvas' starts populating, there is already an entry (which doesn't exist on the replay side as the 'confound' canvas was never emitted)
Copy link
Member

@Juice10 Juice10 left a comment

Choose a reason for hiding this comment

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

Really great catch @eoghanmurray! Thanks for incorporating some nice tests so this won't get broken in the future

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants