29jm / SnowflakeOS

"It is very special"
https://jmnl.xyz
MIT License
316 stars 18 forks source link

Implement basic tb hovering detection system #37

Closed mikumikudice closed 2 years ago

mikumikudice commented 2 years ago

I'm trying to make it more usable by enabling the drag n' drop window system only when the mouse is hovering the title bar. It works, but when you shake the mouse, due to the fact that the mouse slides across the window, it's not too usable. I'm trying to fix the mouse mobility (kernel/src/misc/wm/wm.c) but I'm getting an internal page fault. The fallowing is my error output:

[paging.c] page fault caused by instruction at c01035fb from process 1:
[paging.c] the page at 0000002c wasn't present 
[paging.c] when a process tried to read from it
[paging.c] this process was in kernel mode
[stacktrace.c] stacktrace:
[stacktrace.c]  c0100fa0: paging_fault_handler
[stacktrace.c]  c0102200: isr_handler
[stacktrace.c]  c010aae3: isr48
[stacktrace.c]  c01039c0: wm_mouse_callback
[stacktrace.c]  c0108b60: mouse_handle_packet
[stacktrace.c]  c0108a10: mouse_handle_interrupt
[stacktrace.c]  c0101b40: irq_handler
[stacktrace.c]  c010a9ea: irq15
[stacktrace.c]  c0100000: .
[stacktrace.c]  c0100000: .
[stacktrace.c]  c0100000: .

I also updated the snow_draw_window to accept colors, so then we can use the function anywhere in various ways. Unfortunately it seems to not work properly when used in terminal.c, for example, resulting in a black window.

Any help in any of these two problems would be great.

mikumikudice commented 2 years ago

I didn't opened an issue because my issue is related to my changes, not the repo itself

29jm commented 2 years ago

I'm currently looking at it, will get back to you soon. Random notes:

I really appreciate someone with design ability looking at this area btw

29jm commented 2 years ago

Left a few comments, let me know if you encounter new issues.

Do you think it would be possible to keep the structure of wm_mouse_callback more like it was, just adding one more condition to the "dragging" branch to catch the titlebar? I think it would also help with implementing the fix for the dragging speed issue.

Edit: this page has info on how to debug stuff and might be of help.

mikumikudice commented 2 years ago

I was trying to rewrite the wm_callback due to its slidy behaviour, but if it's better to you I can go back to the original code and just update it. Also, thanks by the debug stuff, it will be quite useful (and sorry by not finding it by myself).

29jm commented 2 years ago

I made a draft version of the dragging fix fwiw, I just put it up on branch dragging-improvs. It works, with some not-smooth behavior when dragging a window close the the screen edges (due to the collision check with edges).

I'd be in favor of having wm_callback stay closer to before if you don't mind, I admit in good part because I know it and understand it well, and that function's job is a bit complicated.

mikumikudice commented 2 years ago

I'm trying to implement now an highlight effect on the title bar (activated when hovered) but I don't know how to get the wm_window_t from a window_t. Is any function that give me back the wm version by the giving id?

29jm commented 2 years ago

Is any function that give me back the wm version by the giving id?

There are no ways to do that right now (no use case), what would that allow you to do if it was possible? edit: feel free to add one if it's clearly needed.

For the highlighting behavior of the titlebar, what do you think of the following way to approach this:

mikumikudice commented 2 years ago

I was planning to simply add a field to the wm_window called is_hovered and use it to tell the snow_draw_window when use the highlight colors or not. But then I noticed that in terminal.c we only have access to the window_t, not the wm one. But the events sounds a good approach to me. I'll try to implement them

29jm commented 2 years ago

Ah I understand. If you want to go for the polling route as opposed to event, a relatively easy way to do it (I think, but fuzzy) would be to add a command to syscall_wm that would return things like metadata about a window, hover for now but could be updated to include other stuff later.

mikumikudice commented 2 years ago

I added two new syscalls (one to get the position of the window of a given id and one to get the hovering status) and implemented the highlight as well. Now I just want to enable paint to paint on click except when clicked on title bar, so if you could explain to me how paint.c works (and how to use it when running) it would be great

29jm commented 2 years ago

Ok so paint uses widgets to render its UI and handle events, unlike say the terminal which as of now draws its UI manually. If you have experience with a library like GTK+, or even Qt to some extent, the UI library in Snowflake OS uses similar concepts. The idea at least in this project is, your app has a top level container, that you can put stuff in including other containers, and in the end what fills those containers will be widgets: labels, buttons, text fields, menu bars, custom widgets, etc. As it stands, the top container is a vertical box (vbox_t), prepopulated with a titlebar_t widget. Whatever comes next in that container will be the app itself, ui_set_root is used to set that.

For paint the whole hierarchy goes like this: vbox_t: hidden root
-> titlebar_t: paint
-> vbox_t: root of UI
-> hbox_t: menu bar
-> button widgets: colors and stuff
-> canvas_t: drawing area

Hopefully after staring at it for a while this mess of a table will make sense :)

The containers are responsible for sizing their content (containers and widgets), and if needed the content also resizes its content etc all the way down the widget hierarchy. Widgets can specify how they like to be sized, e.g. capable of expanding horizontally, vertically, or not, etc. Containers also propagate UI events down to the widget they contain, e.g. you click on a container, container checks what widget was under your cursor, tells that widget (or container) that it got clicked at that spot (the coordinates of the click are converted to local coordinates from the widget's perspective (see the doc on coordinate functions in ui.c)).

How paint works is simply that it has a canvas_t widget that is setup to react to click and move events from the mouse, and because the widget "takes" space in the UI, it can just draw in that space (see canvas.c for details) when it is told to draw by its parent container (i.e. all the way up the chain, by paint.c in the main loop).

The previous limitation with the windows being draggable from everywhere meant that currently you have to click once in the canvas to start drawing, and click again to stop. With your changes to drag windows from titlebars, it should be doable now to draw in more conventional paint fashion, probably with some update to support it in the UI lib.

Take a look at "ui.h" to see how widgets are implemented (it's the usual kind of object oriented C à la gtk) and some tiny doc on it.

I hope this helps get you started on that side of things :)

mikumikudice commented 2 years ago

I did understand how the paint work. I did this side correctly (or at least set everything to). Now I'm facing a problem with the mouse api. I tried to implement an "on release event" but I was not able to set wm.c to handle well dragging, once the click event is set once. My logic (for paint) was: "if clicked then start painting; if released stop to", but the release is set as soon as the click ends and I don't know how to really handle the physical button release. So, if you could give me some light (again) on this problem it would be great.

Also, I tried (and undone most of it) a lot of changes in the code, such as make wm_rect_t and rect_t the same thing (only removed the alias of wm_rect_t), fuse the button and color_button (paint and calc got really messed and bugged), but none of this worked because, sincerely, this part of the code is kinda messy and confusing, so I left this massive change (for example, reimplement the GUI/UI apis to fallow this format) for a new and later PR.

mikumikudice commented 2 years ago

Actually, I think I found a way to do it. I just remembered that the dragging stops right when the mouse releases the button. I'll try to do something on this tomorrow

29jm commented 2 years ago

Making drag events a thing in a clean way in the wm will take some effort, for sure. Having DRAG_START, DRAG_MOVE and DRAG_END events sounds useful. Let me know if doesn't want to work and I'll take a shot at it, compare notes.

By the way, I'm curious what problems you ran into merging button_t and color_button_t?
One thing that I know poses problem is to try to extend an existing widget, e.g. struct color_button_t { button_t base, <custom color_t members> };, that just lead to a mess when I experimented with this.
I wouldn't expect problems with using a single widget for a normal and a color (image or whatever) button though, as long as you provide different constructors or a constructor with proper parameters.

mikumikudice commented 2 years ago

Making drag events a thing in a clean way in the wm will take some effort, for sure. Having DRAG_START, DRAG_MOVE and DRAG_END events sounds useful. Let me know if doesn't want to work and I'll take a shot at it, compare notes.

By the way, I'm curious what problems you ran into merging button_t and color_button_t? One thing that I know poses problem is to try to extend an existing widget, e.g. struct color_button_t { button_t base, <custom color_t members> };, that just lead to a mess when I experimented with this. I wouldn't expect problems with using a single widget for a normal and a color (image or whatever) button though, as long as you provide different constructors or a constructor with proper parameters.

The buttons merge made the paint and calc windows turn black. The rect/wm_rect made the entire system go crazy and messed up. I think I can do it better, but when actively working for this as main goal.

mikumikudice commented 2 years ago

Also, I just did it. Now it's just a matter of polishing. The click sometimes isn't detected, and the cursor flicks on the canvas. Put that aside, everything works as desired

29jm commented 2 years ago

Hoo it's awesome to see it work that way! Great job. I'll give it a proper review tomorrow.

mikumikudice commented 2 years ago

Despite paint now highlights the buttons on strange colors (I'm not sure why once I'm just adding 0x030303 to them) everything keeps working as should (or better) and all changes requested have been effectuated

mikumikudice commented 2 years ago

No, wait. Dragging stopped to work properly. Quick fix soon. Edit: done! Ready to merge

29jm commented 2 years ago

Thanks for addressing all that! I will bother you tomorrow for a few more small things (sorry ^^'), I will have time to test it better also, but this is shaping up, very cool.

mikumikudice commented 2 years ago

No problem, I'm here to help

mikumikudice commented 2 years ago

Everything is set up and up to date. Thanks for lighting up for me some of my faulty acts. Now, for sure I guess, we're ready to merge

mikumikudice commented 2 years ago

I'll fix that tomorrow. Sorry, I think my last test before push had not been done with a make clean.

mikumikudice commented 2 years ago

My current local build, now I fixed every issue, works perfectly (as well as it's possible), including the save / clear buttons from paint. I'm just fixing some other minor stuff and then I'll push everything

mikumikudice commented 2 years ago

I didn't entered GitHub since I pushed the last commit, sorry. I'll fix everything (hopefully the last time) tomorrow.

By the way I always say "tomorrow" because I always see the GitHub notifications on mobile at night

29jm commented 2 years ago

I didn't entered GitHub since I pushed the last commit, sorry. I'll fix everything (hopefully the last time) tomorrow.

Take the time you need :)

By the way I always say "tomorrow" because I always see the GitHub notifications on mobile at night

We have like 7h of time zone lag I think!

mikumikudice commented 2 years ago

I did everything, except fixing the title bar, but I know what's the problem (I thought it could happen but ended forgetting about that): the height-most UI child objects are covering the title bar text, that is being drawn UI_TB_HEIGHT pixels bellow the main position. Setting to all widgets to be drawn that amount of pixels bellow to will fix the problem (but I don't know how to fix it, so I'll put you in charge of this \:p). Again, thanks for the help and by accepting my changes

29jm commented 2 years ago

Will merge this weekend (need to take some notes), and hopefully get to do some snowflake work while I'm at it as all of this gave me a few ideas. Thanks for your work and patience :)

29jm commented 2 years ago

Merged as promised :)

edit: made a few fixes on master, try them out!