Preliminary refactoring patches for QOI #730
Conversation
|
For API breaking refactors, can you use |
| pub width: NonZeroU16, | ||
| pub height: NonZeroU16, | ||
| pub format: PixelFormat, | ||
| pub data: BytesMut, |
There was a problem hiding this comment.
question: What is the benefit of using BytesMut over Vec<u8> at this place?
There was a problem hiding this comment.
it's not nessarily a Vec, we may have shared memory. Although I realize shared memory may be problematic too... Perhaps a whole copy is inavoidable..
There was a problem hiding this comment.
User has to be careful that shared memory will be owned by the server. But we should still allow shared memory to avoid copy.
There was a problem hiding this comment.
I assume that at some point freeze() or alike is called, and memory is actually shared then?
This sounds good to me, but as you point out, let’s be careful about how we use this: we experienced issues in the past where the way BytesMut was used caused too much memory being allocated and freed, resulting in both performance and memory usage being poor. (Big chunk of memory may also never get freed, as long as a view on it is remaining, even if it’s a very small view. And how BytesMut is used is also very relevant.)
I assume you are measuring the CPU and memory usage, and have a good insight. It would help avoid regressions to add some in-line comments to document the decision.
Dealing with multiple formats is sufficiently annoying, there isn't much need for awkward image layout. This was done for efficiency reason for bitmap encoding, but bitmap is really inefficient anyway and very few servers will actually provide bottom to top images (except with GL/GPU textures, but this is not in scope yet). Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
This will hold the updated bitmap data for the whole framebuffer. Signed-off-by: Marc-André Lureau <[email protected]>
This is more idiomatic, and thus less confusing. Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
The bitmap encoder dispatching code was becoming convoluted and the same struct was handling PduEncoding and various bitmap encoding handling. Instead, split UpdateEncoder in different types and concerns. Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
It should reflect client drawing state. In following changes, we will fix it to draw bitmap updates on it, to keep it up to date. Signed-off-by: Marc-André Lureau <[email protected]>
subset of commits taken from #670