fschutt / azul

Desktop GUI Framework
https://azul.rs/
Mozilla Public License 2.0
5.87k stars 218 forks source link

Information about provenance of callbacks #20

Closed quatrezoneilles closed 5 years ago

quatrezoneilles commented 5 years ago

I know I'm being impatient, but azul has enough features for me to start using it now, except for one little thing. In the current last chapter of the guide there is the following

... callback information that is necessary to determine what item (of a bigger list, for example) was interacted with. Ex. if you have 100 list items, you don't want to write 100 callbacks for each one, but rather have one callback that acts on which ID was selected. The WindowEvent gives you the necessary information to react to these events.

I cannot stress enough how important that is. If you have several sliders, you want to make it easy to link each one of them with the value that it controls in the data model. Right now one way of doing this is by customizing your callback with a macro, but that won't work if you generate the Dom programmatically. The chapter I quoted sketches another way, by trawling through the Dom tree, and it would be rather easy to work out a

impl<T> Dom<T> {
    fn get_node_by_node_data_id (&self, ... ) { ... }
}

using the current state of the code. But it is obviously something that fschutt is leery about, and I agree that only the rendering code should enter the Dom kitchen.

I would like to suggest the following instead: add an associated IndexType to the Layout trait, and an extra parameter of that type to callbacks. The type is to be used by developers any way they want to access the data model.

I wouldn't be surprised if that is already in the plans. Or something else that's even better.

That's the only extra stuff I want for the 0.1 release (and solving the MacOs bug)

fschutt commented 5 years ago

Yeah I know about this and I'll need to do this for the 0.1 release of course, but my idea was more based on a "querying" system. Basically the WindowEvent should give you back the DomHash, i.e. the hash that uniquely identifies this node, from which the callback was called. Note that DomHashes aren't very persistent across frames, they only serve to identify a rectangle in the current frame, so you can't store them anywhere. I.e. something like this:

fn my_callback(data: &mut AppState<MyData>, event: WindowEvent<MyData>) -> UpdateScreen {
    let window = app_state.windows[event.window];
    // Query some information about the hit DOM rectangle
    let dom_query_result = window.query_dom(event.dom_hash);
    // The hit rectangle is the Xth rectangle in the parent
    let item_in_parent = dom_query_result.index_in_parent;
    // You could also query the DOM hash of the parent of this node or the current classes
    // (although I'm not too sure how useful / easily misused that'll become)
    let dom_hash_of_parent = dom_query_result.parent_hash.unwrap();
    // And then query information about the parent (maybe, not sure)
    let dom_query_result_of_parent = window.query_dom(dom_hash_of_parent);
}

Then you can "query" the framework for information about the hit DOM node, i.e. position, size, id, class, as well as the index of the DOM item in the parent (which can be used to identify the Xth slider for example). I wanted to do a querying-based API, because then I can extend the returned struct later on and control what things can and can't be queried.

I do want to keep these things "seperated" and as clean as possible (i.e. so that the callback doesn't depend on the view), but in order for the callback to do useful things, you'd need to have something like this.

Also, the framework will need number variables in the dynamic properties / CSS variables, so that you can for example style the 5th slider button properly. Right now if you make a dynamic property, you can only style all sliders some way, not the Xth slider separately. This is just because the DOM isn't retained across frames, so if you'd "set" some property on a DOM element, it would be gone on the next frame.

Also, text field focus and listening for desktop events (i.e. events where there window doesn't have to be active) are on my to-do list.

fschutt commented 5 years ago

I am however not sure exactly what things you should be able to query, ex. if you should be able to query the ID or classes or CSS properties of the node because that could be misused to make the callbacks dependent on the view, which is something that I'd like to avoid, keeping the view and the callbacks (the business logic) completely seperate.

So for a 0.1 release, my idea was to just include the index in the parent and then see if in the real world something else is needed and add it later. For example, it might turn out that querying for classes is super important - alright, then I can easily add that later.

quatrezoneilles commented 5 years ago

One hour after I sent my post I realized I'd reinvented the wheel. What I called IndexType is known as Msg in both elm and yew. There is the difference that azul would be much less prescriptive in how the type should be used.

So we can talk about two approaches, the "message" and the "query" approach. I think both are needed, and that it's important but not trivial to establish what belongs where.

Information like "I am the fifth slider in the list" belongs to the logical structure of the app, is already known by the code that builds the Dom and including it in a NodeData field is cheap. This is "message" information.

Information of the sort that transforms simple mouse/kbd events into more complex gestures like dragging belongs to the "query" world. In elm there are several libraries for dragging, with different tradeoffs. Something of the sort will probably also evolve in azul.

But with information like "i am the tenth slider in the list, and due to available space it was decided that I should be on the second row of sliders and so I need a different color" it is much harder to decide. I think that the conventional wisdom in immediate mode GUI design is that it should be established by communication between widgets through the data store. But in azul there is a Mutex barrier that might slow down things too much.

I have to think a little more about all this. Hoping my musings are useful.

fschutt commented 5 years ago

Yeah, well you always have to consider if this use-case is common. Generally, when you add a slider for example, you don't add the slider willy nilly. The slider should control something, it has a purpose, it's not just there for decoration. Usually you know how many sliders there are at compile time when building dialogs. For example if you have an [X, Y, Z] slider component, you already know that you have exactly 3 sliders. If you build a checkout dialog to select X amount of items, you know that you have exactly one slider. Even in webdev which is all about building UI, I can't think of a situation where I'd have something like a slider that styles itself depending on which row it is in. Sure, I do want flexibility, but a use-case like "I want a dynamic amount of sliders" is a pretty edge case, I mean so far I haven't seen any (desktop or web) GUI where I can create a dynamic amount of sliders (not saying it's not possible - I just haven't seen it in any production app). It's just - you'd be optimizing for a 1% case, not a 99% case.

Second, you have to take into account that you can't put auto-updating sliders on the heap. This is a technical limitation, has to do with the safety guarantees of default callbacks (i.e. that the slider auto-updates a certain value - that value has to be on the stack for safety reasons). I'll finish the page on two-way data binding later this week, there will be an explanation on why this limitation exists.

You can also always hit-test manually from the parent DOM element if you need to, it's not that hard. Ex. if you have a parent element of the sliders, and you know that all your sliders align vertically and are 50px high each (which is something that you can store in your data model and then style dynamically), and the cursor hit the parent element at a y of 120px, you can easily calculate that the third slider was selected. This is how you'd solve the 1% case with the dynamic-amount-of-sliders to work around the stack/heap problem.

Azul generally doesn't work via messages, mostly because messages are highly inflexible, at least in the Rust world - at the beginning, you might make a message to send data between a button and a label, like Msg::Increment, but later on in a more complex app, you need seperate formats of messages, like Msg::DialogMessage(DialogMessage::Dialog1(Dialog1Msg::Increment)). So you are carrying around a whole bunch of stuff that isn't related to the Dialog1 at all. Second, you have the problem that anything can send a message to anything else, essentially "everything is public". By communicating over the application state, you can at least avoid it a bit with public / private visibility in modules, plus you can have unit tests where you can assert that certain callbacks only touch certain fields.

And third, you have the problem that your app sooner or later will start to expect messages in a certain order - say you test a stream of [message a, message b, message c] - and suddenly in production, a user inputs [message c, message a, message b] - now you didn't test that order, so your app crashes. The complexity of combinations to test goes up exponentially - if you have 2 message types, there can be 2 combinations , 3 types, 6 combinations, 4 types 10 combinations, 5 types 25 combinations. And writing 25 tests to test all possible combinations is a pain, so people don't do it.

In azul, it's much simpler you setup some state and then assert that the callback reacts in a certain way, if you setup state A, call callback B, you always get state C - and you can test that callbacks don't touch other fields than they are supposed to. Doesn't exactly solve the exponential-testing problem, but makes it more maintainable.

Elm prides itself on ease-of-debugging - but with azul you shouldn't need to debug in the first place, if a certain state isn't handled by a callback it shouldn't even compile. The philosophy is more that you should rather write unit tests than debug - because debugging is only done once, tests can be repeated. Debugging can't be put into a CI, tests can. So if you have a bug, capture the state of the app or the failing component, write a failing test case and then fix it - now your future maintainers know that this case never fails. If you just debug it and move on, then future maintainers might stumble across the same mistake again and has to debug it again.

including it in a NodeData field is cheap

Well, keep in mind that the NodeData is on literally every DOM node, whether it is used or not. If your application grows, so does your message type, and therfore every DOM node. Larger applications have usually 2000 - 7000 DOM nodes (took those numbers from measuring the number of DOM nodes of outlook.com and github.com). Each byte also needs to be hashed - the NodeData field struct be kept as small as possible. I wouldn't argue against that if it would make the application architecture more reliable... but well, see the reasons above.

That's not to say that you can't build such a messaging system on top of azul (as a wrapper library). You could absolutely do that, make an elm-style message-passing system as a library that wraps the data model. For now I think I'll stick with the querying and state-based system for the reasons above. Also, querying shouldn't be the exception, not the rule. Components like sliders, etc. should generally not need to query anything, they shouldn't know about their position or something like that. That's what I want to avoid, because it couples the business logic tightly to the view.

Lastly, gestures are very complex, because you essentially need to reparent UI components. The general case is simply if let Gesture::InProgress(x, y) = self.gesture { render_dom_with_flying_component(x, y) } else { render_dom_normally() } - gestures should be handled in the state -> view, not in the business logic (the callbacks). The callbacks should just update where the finger is (in a swiping motion). But that's still very far into the future.

quatrezoneilles commented 5 years ago

First I'd like to thank you for taking so much time to answer my naive requests. I will only make short comments now, and try to understand the azul viewpoint better before I add more.

Usually you know how many sliders there are at compile time when building dialogs.

Well here is my counterexample, and I'm shopping for a gui framework that allows me to do it cleanly: I am working on a library for audio DSP. I want the gui code to read a synthesizer interface spec and autobuild a useable panel for it. Styling hints could be added, but raw specs should produce something useable, doesn't matter much if it's ugly. BTW such a thing already exists in the Faust language, in C++ and HTML.

I'm sure there are many potential azul users with similar needs.

Even in webdev which is all about building UI, I can't think of a situation where I'd have something like a slider that styles itself depending on which row it is in.

When I made up this example I thought it embodied what you had in mind in your code above. Guess I misread it.

Waiting impatiently for those guide updates!

fschutt commented 5 years ago

I've thought about this and came up with a better solution than the query system which also works around the stack-based values problem. While I haven't completed the page yet, I've made some diagrams to help you understand what the stack / heap problem is. The rule is that the slider's on_click can't be called automatically from the framework, since we need a &mut DSPSlider somehow to invoke the callback. Otherwise, the framework would be fairly useless, since it can't update any state (which is the whole point of a user interface). Imagine a slider that can't update itself - what's the point right. So to comply with the mutability rules, you can push a StackCheckedPointer during the layout() function plus a callback that goes along with it. Then azul will "auto-update" your slider by just calling the callback on the pointer you gave it:

Azul Default Callback Memory Model Stack Heap

You are allowed to push a pointer to your &DSPSlider + a callback, but only if the &DSPSlider came from the stack. Otherwise, you could potentially push the pointer, then delete the memory and boom, a dangling pointer:

Azul Default Callback - Registering a new default callback

If the DOM node is hit, azul locks the data model, then invokes the callback on the pointer, so that you can update your state in the on_click method:

Azul Default Callback - Invoking default callbacks

(sorry just noticed that the arrow is off in that diagram, should go to the TextInputState, not the TableViewState).

However, this does not work if the pointer would be coming from the heap instead of the stack - because then you could delete the memory for the DSPSlider, while azul still thinks that the pointer is valid and when the rectangle is hit and the callback is invoked - either yout get a segfault or worse. So the on_slider_was_clicked() can't be called automatically by the framework, so how do we call it if we need a dynamic amount of sliders?

Essentially, you should be able to get the index of the item you clicked on... but not from the item itself, but rather from it's parent node (since all DOM nodes have a parent except for the root node). What you really want is to see what nth item was clicked / hit, and then update the state of the list (or Vec) at n accordingly, i.e. my_panels[n].slider += 0.1 or something like that.

What I had in mind was that essentially the parent container has an On::Click event handler that can "delegate" the click event to its children. The reason why this is fairly easy to do is because we already got all the hit test results from webrender, so checking if the child item was hit doesn't involve manual hit-testing and is almost O(1). Sort of like this (pseudo code):

// If heap-allocated objects are involved, need to "delegate" the event from the parent to the child
fn on_click_parent(parent: &mut DSPSliderContainer, info: WindowInfo) -> UpdateScreen {
    // The parent DOM rect was hit, test if any child items were hit and return its index
    if let Some(hit_child) = info.get_hit_child_rect(On::Click, info.dom_hash) {
        // call the childs event handler with a &mut DSPSlider - to the nth item in the Vec
        parent.sliders[hit_child].on_click_child(info);
    }
}

// can't be called automatically by framework if value is on heap - see below
fn on_click_child(child: &mut DSPSlider, info: WindowInfo) - > UpdateScreen {
    child.value += 0.1;
    UpdateScreen::Redraw
}

This essentially "delegates" or "forwards" the onclick event from the parent to the child. Usually, when the child is clicked, the parent is clicked too, since the bounds of the child DOM are inside of the parent DOM (I know, edge cases, but for general, rectangle-based UIs that's true). More general - I can't solve the heap / stack problem because it's something that nearly all frameworks struggle with (view -> model binding), but at least I can make it easy to migitate (like what, 3 lines of extra code).

I think this system will work well, but I'll need to experiment to see if it does. Right now I'm more working on getting spreadsheets and table views to render, but I'll notify you when it's done.

fschutt commented 5 years ago

@quatrezoneilles I've updated the page: https://github.com/maps4print/azul/wiki/Two-way-data-binding

Technically, this issue is closed by https://github.com/maps4print/azul/commit/25dcc41ab9a737409d46e193b25d63ff85c890d3 - although there will be some issues regarding focused / unfocused, etc. events. And it may need more documentation. Feel free to reopen the issue if you think that's not enough for your use-case.

I've also added the list.rs demo that shows how you can write one callback to the the Xth item in the list. The same thing would apply to sliders or other inputs, of course.