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

faster offscreen canvas #122

Closed
totaam opened this issue Dec 4, 2021 · 7 comments
Closed

faster offscreen canvas #122

totaam opened this issue Dec 4, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@totaam
Copy link
Collaborator

totaam commented Dec 4, 2021

Cranking Up Performance in Graphics Intensive Web Apps and Games (Google I/O '17)
OffscreenCanvas : Canvas on worker thread
OffscreenCanvas — Speed up Your Canvas Operations with a Web Worker

@totaam totaam added the enhancement New feature or request label Dec 4, 2021
totaam added a commit that referenced this issue Jan 7, 2022
@totaam
Copy link
Collaborator Author

totaam commented Jan 7, 2022

Merged in the commit above, still TODO:

  • re-use box-colors debugging feature
  • add taskbar height option
  • re-instater check_encodings
  • better frame input throttling
  • it leaks decoders! (never removed)
  • html5 window resizing goes blank #144 is easier to trigger?
    etc..

@totaam
Copy link
Collaborator Author

totaam commented Jan 11, 2022

After fixing a few minor issues, I see a bigger problem: the offscreen decoder paints all screen updates in the order they are decoded and not the order they are submitted which causes visual corruption, in particular with scroll paints but also with images that are slower to decode, and it does not honour the flush attribute either: all updates are shown on-screen immediately instead of grouped together as a single update unit when we reach flush=0.
The regular paint code does a buffer flip and I'm not sure yet how to do the same thing with transferControlToOffscreen.

Fixing these problems may well cost us some of the performance gains.


Ideas:

  • use this method only when there is a single update (flush=0)
  • somehow use webgl double buffering to cheaply copy buffers so we can restore some version of swap_buffers
  • disable scroll so we can keep some form of asynchronous painting but add a fence on flush=0
  • only use offscreen canvas for fast updates, ie: video
  • work on the older decode worker and make it smarter: don't hold the packets, hold a (limited?) number of decoded images so we can start decoding them in parallel - or just fence on flush=0 as per above

@totaam
Copy link
Collaborator Author

totaam commented Jan 26, 2022

I forgot to tag a bunch of important commits with this ticket number: d94a44d, 5818e2a, 20d82a6, c49d12f, 42c6f3d.
In particular the final ones: fbff650 + 91bc852


What still needs fixing:

  • 8bd9b10 : handling all calls to request_redraw somehow
  • 6b2984e : scroll encoding doesn't work with the offscreen canvas, why??

@totaam
Copy link
Collaborator Author

totaam commented Jan 27, 2022

New items:

  • error handling: if things get wedged, we need to flush all pending lists and just start again by asking the server for a refresh - otherwise, screen updates would completely stop
  • request_redraw can just ask the decode worker to do a repaint from the backbuffer now
  • the code would be nicer / cleaner if add_decoders_for_window used a window-decoder object
  • we don't need to honour the paint order if we're using a back buffer, and we can paint to the back buffer as soon as the image is decoded (for the current flush sequence only)
  • video frames can go directly to the front buffer? (ignore vsync? paint the back buffer after or not at all?)
  • we should still use OffscreenCanvas.requestAnimationFrame to get clean vsync repaints

totaam added a commit that referenced this issue Jan 27, 2022
totaam added a commit that referenced this issue Jan 27, 2022
as long as the packet is valid for the current flush sequence
totaam added a commit that referenced this issue Jan 27, 2022
images may be in the process of being decoded when the window is destroyed, just drop the screen update
totaam added a commit that referenced this issue Jan 27, 2022
totaam added a commit that referenced this issue Jan 28, 2022
makes the code more re-usable,
the window now controls the canvas element rather than being given one by the client,
the extra canvas objects are no longer defined when using the new offscreen worker
totaam added a commit that referenced this issue Jan 28, 2022
totaam added a commit that referenced this issue Jan 28, 2022
one is a paint packet option, the other one is a sequence number
totaam added a commit that referenced this issue Jan 28, 2022
totaam added a commit that referenced this issue Jan 31, 2022
@totaam
Copy link
Collaborator Author

totaam commented Jan 31, 2022

Here are some other things that I wanted to look into but I am running out of time:

  • we send back the echo immediately after decoding the image, but the screen update may still take time before landing on screen via requestAnimationFrame, should we store the ack packet and wait for the vsync? Record this delay separately and report it to the server? From the docs, it seems that the delay can be quite long in some cases (ie: page not visible) and we don't want the server to think that the client is hosed.
  • the throttle code should tell the server more explicitly what is expected to happen
  • can we handle B-frames with the VideoDecoder interface?
  • should we buffer flip instead of copying? How?
  • one-shot window updates should just go directly to the front buffer via requestAnimationFrame (major speedup by avoiding one 1 extra pixel copy) - maybe even most updates, except for the ones that have scroll draw packets?
  • restore the encodings validation checks by refactoring the test code from DecodeWorker.js
  • jpega would be very useful to have, especially for rendering some misbehaved applications (ie: browsers) - perhaps copying the alpha channel one pixel at a time into ImageData.data?
  • better measurements of the performance benefits of this new feature

At some point in the (hopefully short term) future, packet loss from #143 will require some interesting changes to the paint code... which will need some help from the server to make it easier to handle.

totaam added a commit that referenced this issue Jan 31, 2022
so we can choose not to use the back buffer
@totaam
Copy link
Collaborator Author

totaam commented Jan 31, 2022

More thoughts on scroll draw packets and the need for a backbuffer:

  • if we only use the backbuffer when we have scroll to paint then how do we handle redraw requests? (copy to the backbuffer when updates stop? but what if the request arrives in between?) - the commit above would make it easier to implement backbuffer on demand, but somehow we're back to some visual artifacts with scroll encoding and after a window resize:
--- a/html5/js/OffscreenDecodeWorker.js
+++ b/html5/js/OffscreenDecodeWorker.js
@@ -37,7 +37,6 @@ class WindowDecoder {
     }
     init() {
         this.back_buffer = null;
-        this.init_back_buffer();
         this.image_decoder = this.new_image_decoder();
         this.video_decoder = this.new_video_decoder();
         this.flush_seqs = [];    //this is the sequence numbers of the flush packets
@@ -300,6 +299,7 @@ class WindowDecoder {
             paint_box();
         }
         else if (coding == "scroll") {
+            this.init_back_buffer();
             for (let i = 0, j = data.length; i < j; ++i) {
                 const scroll_data = data[i];
                 const sx = scroll_data[0],
@@ -368,6 +368,7 @@ class WindowDecoder {
         const ctx = this.canvas.getContext("2d");
         ctx.imageSmoothingEnabled = false;
         ctx.drawImage(this.back_buffer, 0, 0);
+        this.back_buffer = null;
     }
 }
 

The performance is much better though, since we avoid a copy.

  • use getImageData to capture the scroll data from the front buffer: the difficulty here is that we have to wait for the previous flush=0 fence and its associated requestAnimationFrame before we can capture the pixels

Are we missing a call to OffScreenContext.commit()

Edit: merged below, the patch above was painting the wrong buffer!

totaam added a commit that referenced this issue Feb 1, 2022
totaam added a commit that referenced this issue Feb 1, 2022
totaam added a commit that referenced this issue Feb 1, 2022
take a snapshot of the canvas when it stops updating
totaam added a commit that referenced this issue Feb 1, 2022
'scroll' should always be processed first - it was already the case because of the insertion order, but this change makes it explicit
totaam added a commit that referenced this issue Feb 2, 2022
totaam added a commit that referenced this issue Feb 2, 2022
totaam added a commit that referenced this issue Feb 2, 2022
re-create the canvas when we resume
totaam added a commit that referenced this issue Feb 3, 2022
@totaam
Copy link
Collaborator Author

totaam commented Feb 4, 2022

Let's close this.
We can follow up with other tickets for video improvements, jpega (#152).

Also: echo timing (vs waiting vsync), restore encodings validation, etc.

@totaam totaam closed this as completed Feb 4, 2022
totaam added a commit that referenced this issue Feb 9, 2022
keep compressed packets only and decompress when we need them
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant