Skip to content

Conversation

@gberaudo
Copy link
Member

@gberaudo gberaudo commented Nov 13, 2018

The goal of this PR is to split the replays in 2 parts:

  • classes for building instructions (typically used in web workers);
  • classes for executing the instructions (typically used on the main thread).

Plan is:

  • have a replay group instance for building instructions and one for executing them;
  • be able to extract and replace instructions;
  • physically split the builders and executor in separate classes;
  • only pass information in instructions and coordinate array;

Postponed to a later phase:

  • make instructions serializable (remove image creation from the styles, create and cache images in the instruction executor);
  • binary transferable representation of the instructions (will be a lot of work)

@gberaudo gberaudo force-pushed the replay_refactoring branch 2 times, most recently from 482c04b to 2255149 Compare November 13, 2018 14:42
@ahocevar ahocevar force-pushed the replay_refactoring branch 2 times, most recently from c9abff7 to 8c300a6 Compare November 13, 2018 16:27
@gberaudo gberaudo force-pushed the replay_refactoring branch 4 times, most recently from e4fe204 to 8303281 Compare November 15, 2018 08:54
@gberaudo
Copy link
Member Author

@ahocevar, the creation of text instructions is now fully separated from the executor.
I will do some cosmetics cleanups and after that it should be in good shape for merging.

I will work on removing the creation of images from the styles in a different PR as it is not necessary until we have workers and I am eager to see this PR merged.

@gberaudo gberaudo force-pushed the replay_refactoring branch 3 times, most recently from 76ecf7f to d7df45f Compare November 15, 2018 14:13
@gberaudo
Copy link
Member Author

@ahocevar, the PR is now ready for review.

@gberaudo
Copy link
Member Author

@ahocevar, I fixed the pixelratio issue, improved the test and fixed the mismatching executor/builder types.

I also simplified the names: Executor, Builder, ExecutorGroup, BuilderGroup. That is shorter to type and easier to remember.

The last point left is passing all states in the instructions which I will tackle tomorrow.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

Great work, thanks for this! I added a FIXME about not creating the measure function with every execute() call. That can be addressed in a follow-up pull request.

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