Improve PNG decompression implementation#980
Conversation
|
Hey @tosscaster could you test this? I don't have a Web Worker environment to test this in. Or even helping add an automated test would be awesome. |
bf12e79 to
a1e7700
Compare
This comment was marked as outdated.
This comment was marked as outdated.
866f71e to
059a927
Compare
|
okay, added a test. now we actually know for sure that PNG compression works! |
This comment was marked as outdated.
This comment was marked as outdated.
|
The test timeout isn't from the roslibjs side of things. The initial run before the V8 JIT optimizes it only takes 3.7ms, and then subsequent runs take under half a millisecond. |
5a5fa63 to
65e1ec9
Compare
Use fast-png to unify the implementation between Node.js and browser, and unbork Web Worker support (fixes #859)
65e1ec9 to
35db117
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
I'll try to take a look later today |
812c9dd to
35db117
Compare
|
@EzraBrooks I see what's the problem. There was a bug with PNG compression in rosbridge which was fixed some time ago (RobotWebTools/rosbridge_suite#1045). The fix is included in the newest versions of rosbridge which I release a week ago. I tried the example against the newest version of rosbridge in Jazzy and Rolling and it worked. |
|
So does the introduction of this test need to wait til the next sync? |
Unless we change the dockerfile to use ROS testing repository or build rosbridge itself. |
|
Given that the code works, I'm inclined to mark the test skipped and merge this. Are you okay with that? |
|
Note to self: remove pngparse type declaration file here before merging because the dependency itself is gone |
I'm fine with that |
|
There should be a new sync for each ros distro by the end of this month. After the sync and when the docker images are updated, we should be able to reenable the test. |
Use fast-png to unify the implementation between Node.js
and browser, and unbork Web Worker support (fixes #859)