Kernel: Improve early framebuffer scroll speed with in-memory buffer #25998
Conversation
VIJAY-0
commented
Jun 15, 2025
VIJAY-0
commented
- Introduced (u8 inMemBuffer[]) as a shadow RAM buffer mirroring VRAM layout
- Updated scroll logic to use memmove on RAM buffer instead of reading from VRAM
- After scrolling, flush entire RAM buffer to VRAM using memcpy
github-actions
bot
added
the
👀 pr-needs-review
|
Hello! One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the |
|
First of all: Welcome to the project! |
|
hi , |
|
What issues are you encountering? In general testing builds through the CI is discouraged, as it is for example quite slow and will trigger the BuggieBot quite often, |
VIJAY-0
force-pushed
the
fix-framebuffer-scrolling
branch
from
1555d0e to
d29a8be
Compare
|
@Hendiadyoin1 |
|
Current issue is that the code is malformatted, |
Avoid slow framebuffer reads during early console scrolling by maintaining an in-memory buffer. Scrolling now memcpy's full lines from this buffer to the framebuffer, replacing character-wise copies. Fixes SerenityOS#25960
VIJAY-0
force-pushed
the
fix-framebuffer-scrolling
branch
from
d29a8be to
d142881
Compare
VIJAY-0
changed the title
|
|
||
| memmove(m_in_memory_buffer, m_in_memory_buffer + framebuffer_pitch(), (height() - 1) * framebuffer_pitch()); |
There was a problem hiding this comment.
Please test your code before submitting a PR. This only scrolls by 1 pixel, not by 1 entire line.
| int m_in_memory_buffer_size; | ||
| u8* m_in_memory_buffer { nullptr }; |
There was a problem hiding this comment.
Please use a FixedArray. This code is currently very C-like.
| if (!m_in_memory_buffer) { | ||
| m_in_memory_buffer_size = height * framebuffer_pitch(); | ||
| m_in_memory_buffer = (u8*)kmalloc(m_in_memory_buffer_size); | ||
| memset(m_in_memory_buffer, 0, m_in_memory_buffer_size); | ||
| } |
There was a problem hiding this comment.
Double-buffering the framebuffer was not quite what I meant in my issue.
I think we should have a buffer containing the characters (not the pixels) currently on the screen.
Storing all pixels in a back buffer would also work, but is quite wasteful.
| @@ -225,6 +225,11 @@ GenericFramebufferConsoleImpl::FramebufferOffset GenericFramebufferConsoleImpl:: | |||
| return { (&framebuffer_data()[x * sizeof(u32) * (m_glyph_columns + m_glyph_spacing) + y * m_glyph_rows * framebuffer_pitch()]) }; | |||
| } | |||
|
|
|||
| GenericFramebufferConsoleImpl::FramebufferOffset GenericFramebufferConsoleImpl::inmembuffer_offset(size_t x, size_t y) | |||
There was a problem hiding this comment.
This function is not necessary with this current approach (storing the pixel data in a second buffer).
Also, inmembuffer is not really a good name that fits our coding style. What you made here is essentially a back buffer (https://en.wikipedia.org/wiki/Multiple_buffering#Software_double_buffering).
