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

WIP Compositor #52

Merged
merged 22 commits into from
Dec 12, 2018
Merged

WIP Compositor #52

merged 22 commits into from
Dec 12, 2018

Conversation

roblabla
Copy link
Member

@roblabla roblabla commented Dec 3, 2018

  • Implement a compositor, named vi.
  • The clock uses it.
  • It's broken.

Fixes #40

@todo
Copy link

todo bot commented Dec 3, 2018

Safety. We need to guarantee that, while this slice exists, the

https://github.com/roblabla42/KFS/blob/3d0f9b0284ddf1dcd86d2bb9a35330c28e117d66/vi/src/main.rs#L84-L94


This comment was generated by todo based on a TODO comment in 3d0f9b0 in #52. cc @roblabla.

@todo
Copy link

todo bot commented Dec 3, 2018

Alpha blend!

https://github.com/roblabla42/KFS/blob/3d0f9b0284ddf1dcd86d2bb9a35330c28e117d66/vi/src/main.rs#L105-L115


This comment was generated by todo based on a TODO comment in 3d0f9b0 in #52. cc @roblabla.

@todo
Copy link

todo bot commented Dec 6, 2018

Vi: Implement alpha blending

Vi currently does not do alpha blending at all. In the interest of pretty transparent window, this needs fixing!


https://github.com/roblabla42/KFS/blob/1b914d65646351f62d521f34f11bd9bc9baee98e/vi/src/main.rs#L105-L115


This comment was generated by todo based on a TODO comment in 1b914d6 in #52. cc @roblabla.

@todo
Copy link

todo bot commented Dec 7, 2018

Re-enable test_threads

Test threads has been disabled when VBELoggers were removed, as they would need the ability to get the terminal via an argument somehow.


https://github.com/roblabla42/KFS/blob/c6cff39103bfea0951dfb7917ad6192fd7b14e5e/shell/src/main.rs#L98-L104


This comment was generated by todo based on a TODO comment in c6cff39 in #52. cc @roblabla.

@roblabla
Copy link
Member Author

roblabla commented Dec 7, 2018

So, this works. It's slow though. Very, very slow.

@todo
Copy link

todo bot commented Dec 7, 2018

Safety of the vi::draw IPC method

When calling vi::draw, vi reads from the shared memory. There is no borrow-checking mechanism in place to ensure that the other process does not mutate it while this happens. Maybe we should have some kind of cross-process mutex? How to implement this properly?


https://github.com/roblabla42/KFS/blob/8ad277ff0eac490ba222d1e02dcd3fc7ac1fa8c1/vi/src/main.rs#L86-L96


This comment was generated by todo based on a TODO comment in 8ad277f in #52. cc @roblabla.

@todo
Copy link

todo bot commented Dec 7, 2018

Vi: Heavy flickering.

When drawing the whole screen, we can see some extreme amount of flickering on the screen. We should look into better compositing algorithms, maybe we don't need to redraw the whole screen all the time? And also, look into VSYNC.


https://github.com/roblabla42/KFS/blob/8ad277ff0eac490ba222d1e02dcd3fc7ac1fa8c1/vi/src/main.rs#L149-L159


This comment was generated by todo based on a TODO comment in 8ad277f in #52. cc @roblabla.

@Orycterope
Copy link
Member

Closes #40

vi/src/main.rs Outdated Show resolved Hide resolved
vi/src/main.rs Outdated Show resolved Hide resolved
vi/src/vbe.rs Outdated Show resolved Hide resolved
vi/src/vbe.rs Outdated Show resolved Hide resolved
vi/src/vbe.rs Outdated Show resolved Hide resolved
libuser/src/window.rs Outdated Show resolved Hide resolved
libuser/src/window.rs Show resolved Hide resolved
libuser/src/window.rs Outdated Show resolved Hide resolved
libuser/src/terminal.rs Outdated Show resolved Hide resolved
libuser/src/terminal.rs Show resolved Hide resolved
@todo
Copy link

todo bot commented Dec 10, 2018

Terminal - Get window size from vi

Terminal defines a fullscreen size as 1280 * 800, but should really get it from vi instead.


https://github.com/roblabla42/KFS/blob/437d3291fc9b7343f0ff511c6b50d0e227e6c5b8/libuser/src/terminal.rs#L70-L80


This comment was generated by todo based on a TODO comment in 437d329 in #52. cc @roblabla.

- Implement a compositor, named vi.
- The clock uses it.
- It's broken.
- VBELogger::new() now takes a WindowSize, allowing multiple patterns for
  creating windows.
- Bugfix: BPP should be 32, not 4.
- Move clock::vbe to libuser::window and libuser::terminal
- Remove clock::logger
- Lock framebuffer once at the start of the loop
- Copy framebuffer verbatim, don't attempt interpreting pixels
- Better TODOs
- Remove closed buffers from the global.
- Dropping a ClientSession now sends the Close message
- On the server, handle the Close message by removing the associated Object
- Implement Drop on vi::IBuffer
- Bugfix: use the correct type if we pass a token in new_request/new_response
The old behavior was weird: it would only crop left/top to 0. Now we ensure the
returned output is cropped to the given width/height as well.
- VBE's framebuffer is now an array of VBEColor
- Asserts that x/y is in framebuffer range
- Lock framebuffer once per draw to avoid deadlocks.
- Documentation improvements.
- Document various undocumented parts.
- Panic in get_px_offset if out of bound
- Work around a weird assumption that we have more than 1 line available.
- framebuffer is now an array of color.
Draw happens in a backbuffer, which gets memcpy'd into the framebuffer.
This fixes the flickering issue.
Use the generic method to get the pixel offset in the framebuffer.
@roblabla
Copy link
Member Author

@Orycterope Fixed everything. It's ready for review again.

vi/src/main.rs Outdated
let dtop = min(max(self.top, 0) as u32, framebuffer_height);
let dleft = min(max(self.left, 0) as u32, framebuffer_width);
let dwidth = min(framebuffer_height - dtop, self.height);
let dheight = min(framebuffer_width - dleft, self.width);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If window is far to the left:

let self.left = -500;
let self.width = 4;

let dleft = min(max(-500, 0) as u32, framebuffer_width); // = 0, this is fine
let dwidth = min(framebuffer_width - 0, 4); // = 4. Wrong.

The window was completely out of the screen, but this would draw a 4 px window at the left of the screen.

libuser/src/window.rs Outdated Show resolved Hide resolved
libuser/src/window.rs Show resolved Hide resolved
vi/src/vbe.rs Show resolved Hide resolved
vi/src/main.rs Outdated Show resolved Hide resolved
- More pedantic tests.
- Also, some oryctalgorithm.
Copy link
Member

@Orycterope Orycterope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally

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.

2 participants