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

move image addon to base repo #4577

Merged
merged 15 commits into from Aug 1, 2023
Merged

move image addon to base repo #4577

merged 15 commits into from Aug 1, 2023

Conversation

jerch
Copy link
Member

@jerch jerch commented Jun 27, 2023

Moves the image addon code over to the base repo.

TODO:

  • move code
  • adjust CI scripts to run inwasm task automatically
  • integrate with demo
    • pass API tests
    • add ctor & image link options to demo
  • pass all unit tests
  • pass linter

@jerch
Copy link
Member Author

jerch commented Jun 27, 2023

@Tyriar Thats interesting - since I removed the internal worker I dont need to patch eslint anymore. Before with the worker for image decoding, eslint always would run into OOM w'o patching, no clue why. Could it be, that eslint expands the whole project tree for different lib settings, thus doubling the mem load for a separate webworker.d.ts setup?

@jerch jerch marked this pull request as ready for review June 27, 2023 13:05
@jerch
Copy link
Member Author

jerch commented Jun 27, 2023

@Tyriar Read for review. The code is the same as the last patch release (0.4.2), except the license entries (all adjusted).

@Tyriar Tyriar added this to the 5.3.0 milestone Jun 27, 2023
@Tyriar Tyriar self-assigned this Jun 27, 2023
@jerch
Copy link
Member Author

jerch commented Jun 27, 2023

@Tyriar Not sure whats going on with azure atm, both macos and windows runner give me different timeout issues. Tried to mitigate them somewhat, not sure if they are all gone now...

@jerch
Copy link
Member Author

jerch commented Jun 27, 2023

@Tyriar Those timeout issues with heavy load CI runners are really nasty. Its a bit tricky to wait for the state change in the remote browser engine. Tried to replace several waits with pollFor, sadly idk how to express "ensure that value xy does not change" with it. We kinda lack an event from the render logic telling us, that all pending write buffer data was finally applied to the output states. How do you work around that issue in the canvas and webgl renderer tests?

@jerch
Copy link
Member Author

jerch commented Jun 27, 2023

Jesus, the failing tests are really annoying, all versions I tested above pass locally just fine, grrr. 🤯

@Tyriar
Copy link
Member

Tyriar commented Jul 13, 2023

@jerch good to merge when you're ready as this is all pretty separated from the core code. We have a few more changes from the old repo to include as well is all.

@jerch
Copy link
Member Author

jerch commented Jul 13, 2023

@Tyriar I will go though your comments in jerch/xterm-addon-image#55 and transfer the missing changes here as well before merging.

@Tyriar Tyriar merged commit f6738ec into xtermjs:master Aug 1, 2023
8 checks passed
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.

None yet

2 participants