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

Make PGui nice #1603

Open
Tracked by #1636
rdb opened this issue Jan 23, 2024 · 21 comments
Open
Tracked by #1636

Make PGui nice #1603

rdb opened this issue Jan 23, 2024 · 21 comments
Labels
discussion Issues opened primarilly to start a discussion enhancement

Comments

@rdb
Copy link
Member

rdb commented Jan 23, 2024

There are a couple of problems with DirectGUI:

  • Its API is unintuitive
  • It is written in Python, so not available to C++ users
  • The defaults are bad
    Inevitably, every GUI-heavy application ends up writing their own wrapper around DirectGUI.

DirectGUI is based on PGui, which implements most of the functionality, but it is a bit harder to use, and undocumented.

Rather than keep investing in DirectGUI, or in an altogether new GUI system, which would be a lot of effort and add to the maintenance burden, I would suggest that we take a look at improving PGui, which does work and we are already obligated to maintain. Someone should spend some time trying to use PGui, identify the pain points, and either suggest ways we can improve its API (preferred) or write a new (thin) layer over it. Then we can document it and write sample code for it, and promote it over DirectGUI.

Initially, I would like to keep the scope of this project manageable and not try to add too many nice-to-have high-level features (like gamepad navigation), though thought should be given to making these kinds of features easier to integrate in the future.

@rdb rdb added enhancement discussion Issues opened primarilly to start a discussion labels Jan 23, 2024
@BMaxV
Copy link
Contributor

BMaxV commented Jan 24, 2024

https://github.com/BMaxV/basic_panda3d_PGUI

Here are some basic things to get started with PGUI. I will try to expand the example to include binding functions to buttons, researching how hover in / hover out works.

If that's done, everything else should be mostly trivial to build from the pieces.

@Wizzerinus
Copy link
Contributor

Wizzerinus commented Jan 25, 2024

What I'm seeing so far which is problematic for pgui usability:
*Bold text - serious problems, non bold - can live with this

  • setup call is not very intuitive
  • the name thing being mismatched with the text, confusing
  • (will expand more as the example is expanded)

@BMaxV
Copy link
Contributor

BMaxV commented Jan 25, 2024

More reading and notes:

There are actually lots of comments in the code that are pretty good, so just reading the source works.

Buttons have a .is_button_down() that could be used to reverse mcgyver events if we want to.

Concerning keyboard and mouse input, there seem to be mouseWatcher[something] classes and PGmouseWatcher[something] classes and the one we're using with Showbase is the PG one, though I'm not really sure what the differences are yet. See the setupMouse function in Showbase for where that is done.

PGItem defines these functions, so I'm assuming there is some loop that checks if regions are entered or quit or if the keystroke happened with the mouse in the region and so on. Which, again could be rewritten if it's really as simple as x1 < mousex < x2.

  virtual void enter_region(const MouseWatcherParameter &param);
  virtual void exit_region(const MouseWatcherParameter &param);
  virtual void within_region(const MouseWatcherParameter &param);
  virtual void without_region(const MouseWatcherParameter &param);
  virtual void focus_in();
  virtual void focus_out();
  virtual void press(const MouseWatcherParameter &param, bool background);
  virtual void release(const MouseWatcherParameter &param, bool background);
  virtual void keystroke(const MouseWatcherParameter &param, bool background);
  virtual void candidate(const MouseWatcherParameter &param, bool background);

MouseWatcher::get_over_regions seems to be the calculating which region the mouse is in. Which means, we can also look at that see how that is passed on. It might be easier to

my_frame=Frame(my_signal_name)
...
# mouse watcher does stuff and would emit my_signal_name
event_handler.accept(my_signal_name)

DirectGUI uses PGItem::within_region and that rabbit hole ends at PGitem.I :

/**
 * Returns the prefix that is used to define the within event for all PGItems.
 * The within event is the concatenation of this string followed by get_id().
 */
INLINE std::string PGItem::
get_within_prefix() {
  return "within-";
}

So it's literally an arbitrary string that's somewhat hardcoded via the id and then passed through lots of functions. And then used by the event system.

For ScrollFrame there seems to be recompute_canvas function that does the clipping.

@BMaxV
Copy link
Contributor

BMaxV commented Feb 1, 2024

Just had a thought, that it would be really interesting to do on the fly animations for some UI elements and those could be built into the lower level as well. We can obviously build these ourselves, but there is no reason why some of the simpler ones can't be built in.

Like, periodic flashing to draw attention by animating the color value or scale of UI objects.

Like a 10% increase in scale and "value" in HSV colors, on hover to make the UI more... alive? Interactive? Also a "on hover" sound would be nice. I don't think that's built in yet.

@kamgha
Copy link
Contributor

kamgha commented Feb 1, 2024

Also a "on hover" sound would be nice.

There is rolloverSound and clickSound in DirectGUI:

('rolloverSound', DGG.getDefaultRolloverSound(), self.setRolloverSound),
('clickSound', DGG.getDefaultClickSound(), self.setClickSound),

Like, periodic flashing to draw attention by animating the color value or scale of UI objects.

Doing that with the interval system is more flexible. You can always inherit the widgets and extend their functionality.

@rdb
Copy link
Member Author

rdb commented Feb 1, 2024

Let's focus on the fundamental features of a UI system before we discuss adding additional features.

@BMaxV
Copy link
Contributor

BMaxV commented Feb 1, 2024

I played around some more with the PGUI script I linked above.

I managed to get the hover_in hover_out events to work for buttons. As well as clicks. Strangely, the exact same syntax does not work for frames.

All the necessary events are in DirectGuiGlobals, but that's more of a "how to do it", it's not where they are defined. But that doesn't really matter if we just want to use them. And conceptually, there is nothing wrong with having the "button click event" managed by an event manager object. The difference between button.accept(hardcoded signal,function,args) and eventmanager.accept(signal+button_id,function,args) is minimal and not difficult.

I tried the scrolled frame and entry box, both of which work pretty much as expected as far as I can see. The source is again well commented or the variable names are well named, so it's not difficult to figure out what to do with them.

The text_entry also has the familiar get_text and other functions.

I don't think the waitbar has any value whatsoever, since you can just use scale or set frame dimensions directly anyway.

Which leaves the question of how the scroll frame does the masking.

@rdb
Copy link
Member Author

rdb commented Feb 9, 2024

I had a mini-brainstorm with @janEntikan after looking at BMaxV's sample and here are some thoughts:

  1. The API with setup() is awkward, and putting everything in the constructor a la DirectGUI is also problematic, so everything should just be directly settable using properties:
btn = PGButton()
btn.label = "label"
  1. With regards to sensible defaults: the nice thing about PGui currently crashing if you don't use setup() is that we can modify these classes to have sensible defaults, but setup() would switch to the current defaults, so we keep compatibility and setup() would be deprecated
  2. attach_new_node and creating a NodePath for everything is unwieldy, but fortunately you can just use node.add_child and forget about thinking about NodePaths. We could even turn it into a one-liner with something like this:
btn = frame.new_child(PGButton)

You might suggest we need a NodePath for positioning, but...

  1. I know I said we're not adding features, but lack of proper layouting is a major pain point about the current GUI system... we had the idea of introducing LayoutNode that simply organized all its children along a given axis based on their bounding volume and some extra properties, eg. a few borrowed from flexbox. This might not have to be significantly difficult to implement, and may be generalised to also work in 3D space. This would take care of all node positioning, so that manual positioning is no longer needed. It also creates a more defined linear order of elements than random positioning, which helps with adding keyboard navigation later on.

It might be good to also give some thought to how we might implement the following in the future, so we don't lock ourselves in with decisions that we regret later on, even though it's out of scope for now.

  • High-DPI displays, see Better support for high-DPI screens needed #426.
  • Keyboard navigability - something (MouseWatcher?) needs to be aware of the spatial ordering of UI items. I think something based on a hierarchical traversal would do the trick, but this would need to be worked out later.

@Wizzerinus
Copy link
Contributor

If you are interested I actually implemented such a layout system a few days ago. It's a bit dirty code wise but gets the job done. One problem with it though is how there's not a good way to get the bounding box of a directgui element, but if there is such a way for pgui should be pretty easy

@rdb
Copy link
Member Author

rdb commented Feb 9, 2024

Sure, I'd be interested in taking a look.

We could fix the bounding box issue, by properly implementing calc_tight_bounds for PGItem - or we could base it on the (computed) frame, which does limit the applicability of such a LayoutNode a little bit.

I suppose there should ideally also be a way to grow the frame based on the available remaining space - either this could be done by scaling (which requires something clever to counteract the scale for the contained text) or we need to base it on the frame after all.

@Wizzerinus
Copy link
Contributor

The main issue with the bounding box is with how DirectGUI code works.

  • For example, the text of DirectLabels is not inside the said DirectLabel, but is instead added as a component that does not count towards the label's bounding box and has to be accounted for manually by reading a name-mangled property of the label (_DirectGuiBase__componentInfo).
  • A similar issue happens with the dropdowns, but in that case it's not a component that matters but a child node which also isn't accounted for in either getBounds or getTightBounds.
  • Finally, there is separate code for OnscreenText, OnscreenImage and PGItem which has to be checked separately.

This all leads to a pretty clunky and unsound code. PGItem also erases its DirectGUI information due to being a Python extension for a C++ class, which means the component information is being forgotten unless the widget is looked up in some undocumented array.

For PGItem's I believe it's not too hard to check, but don't know whether subnodes are enabled automatically.

While designing the system we decided that the automatic scaling is out of scope of such a tool, although it's possible to calculate margins automatically, the thing is, Panda3D does not really have the notion of a viewport, or the "hard-set width and height". There is not a way to, for example, split the aspect2d into two frames taking 40% and 60% of the app's width, and make sure the proportion is always intact, unlike something like CSS, which means auto-scaling makes no sense in this scenario. (I do like flexboxes a lot, but it's a different paradigm from what Panda currently has)

The code I wanted to show is here. Not the cleanest code I've ever made, but does the thing.

@BMaxV
Copy link
Contributor

BMaxV commented Feb 10, 2024

The API with setup() is awkward, and putting everything in the constructor a la DirectGUI is also problematic, so everything should just be directly settable using properties:

btn = PGButton()
btn.label = "label"

I'm against this. I would much prefer a good constructor (which is where I would look for default values too).

The biggest reason is that it's not immediately obvious which attributes are available to be set and what their wording is. Is it "text", "display", "content", "label" or the same thing with capitalization? Or "button+whatever"? Because setter functions are "functions", it is obvious that they actually do something and change the object through that action. They are easy to document, I'm assuming they are also easy to keep the same for python and C.

My go to negative example is blender, because they use "pos", "loc" and "position" all different depending on context. One of them was the game engine, one of them was regular 3d view, and I think a different one for vertices and objects, but I don't remember which was which.

It's one of the absolutely awful parts of DirectGui that you can and have to change e.g. frame color by using frame["frameColor"]=vec4 .

I know I said we're not adding features, but lack of proper layouting is a major pain point about the current GUI system... we had the idea of introducing LayoutNode that simply organized all its children along a given axis based on their bounding volume and some extra properties.

I don't think it's a pain point? Where is it a pain point? Who specifically is having this problem?

Just use DirectGUI to do this, because that's exactly what it was built for. All the "initialize options" functions you have perhaps wondered about, are exactly that kind of system that crawls the parent/child tree for attributes and rebuilds things according to the parent's style.

Besides, the coordinate system does the work for us. There are even methods for detecting screen resolution for us that at least I used to detect and shift around things so that they are attached "to the right side of the screen with some padding". It is very easy to build and arrange things via a loop and the programmer can even define their own curves if a simple line is not enough.

Any kind of layout system preemptively needs to be flexible and easy enough to do everything and never get in the way.

Keyboard navigability - something (MouseWatcher?) needs to be aware of the spatial ordering of UI items. I think something based on a hierarchical traversal would do the trick, but this would need to be worked out later.

This is easy, needs no specific thought and it should not set it as a default for the programmer by panda, because you cannot know in advance what Frames are going to be used as.

It is easy in the sense that you can just put all the intended frames/buttons in a list, define "up", "down" and "enter", and use the same event keyword we are using for the regular event manager to activate the respective function. It's just that "enter" would dynamically pick which one that is based on where the "cursor" is in the list.

I can build one if you like.


The one thing that I have not seen done well yet in Direct or PGUI, and that I have absolutely no idea to build is something like these heatlhbars and for Apex everything actually:

https://www.gameuidatabase.com/uploads/Agents-of-Mayhem06242021-105126-91152.jpg

https://www.gameuidatabase.com/uploads/Apex-Legends07152020-072204-49086.jpg

Because they require flexible textures, right now, textures are inherited from the scale of the frame, so if you rescale the frame, you also change the scaling the texture as well. Which means you "squish" the texture if you scale the frame on the x axis. Same for Frame size. If I'm wrong about this, I'd appreciate documentation on it :)

And they require some kind of transparency or masking which, I've already mentioned before.

Also take a look at this:

https://poe.4fansites.de/bilder/guides/poe-guides.jpg

The red health orb to the left, with the white energy shield over it, and the blue mana to the right.

  • follows a curve
  • has a border
  • is transparent
  • has these "cap" elements that really draw attention to where the dynamically changing line ends.

I think those kinds of functions have to be the goal.

Games can get away with lots of low fidelity art, but the UI has to be good.

@rdb
Copy link
Member Author

rdb commented Feb 10, 2024

One more thing I forgot in my previous comment...

  1. Event handling might be considered a bit clunky. When using it from Python, it relies on automatically generated event names, like "click-pg123", so you can do this:
do.accept(btn.click_event, my_handler_func)

Do we think this is fine, or is exposing names like "click-pg123" considered too clunky?

In C++, you can create a notification delegate by inheriting from PGItemNotify, which we could extend to Python, so you can do:

class EventNotify(PGItemNotify):
    def item_press(self, item, param):
        print("button", param.button, "clicked")

btn = Button()
btn.notify = EventNotify()

That's very powerful, but that's a lot of code to do something simple... an alternative is allowing you to specify an event name, which is a bit better:

btn.label = 'Start Game'
btn.click_event = 'start-game'

do.accept('start-game', start_game)

We could also consider adding a bind() function similar to what DirectGUI has, so you can bind the handler directly to the button:

btn.bind('click', start_game)

@BMaxV
Copy link
Contributor

BMaxV commented Feb 10, 2024

btn.click_event = 'start-game' would be ideal.

I don't think we need a .bind(). I think it's more helpful, to be transparent about how it works and use eventhandler.accept(button.clickevent, function), especially when learning but also when reading the code.

@rdb
Copy link
Member Author

rdb commented Feb 10, 2024

I'm against this. I would much prefer a good constructor (which is where I would look for default values too).

Items may have many properties to set. Aside from the code style arguments (which may be a matter of taste, but I find typical DirectGUI code with a dozen constructor arguments quite ugly), we're trying to make the API similar between Python and C++, which doesn't have keyword arguments.

The biggest reason is that it's not immediately obvious which attributes are available to be set and what their wording is. Is it "text", "display", "content", "label" or the same thing with capitalization? Or "button+whatever"?

Surely this is the same with constructor arguments or properties - in both cases you need to look up what the properties are called in the API reference. There's no capitalization if we're just consistently following code style conventions, lower_snake_case for everything, as in PEP8. Things like button_frameColor are a DirectGUI-ism, we shouldn't copy that.

Just use DirectGUI to do this, because that's exactly what it was built for. All the "initialize options" functions you have perhaps wondered about, are exactly that kind of system that crawls the parent/child tree for attributes and rebuilds things according to the parent's style.

We're trying to replace DirectGUI here.

DirectGUI doesn't have layouting features--I don't think we're talking about the same thing. I'm talking about something like a GtkBox or NSStackView or wx.Sizer or QVBoxLayout or (insert equivalent for any UI system of choice here). These don't just align elements in a row to a corner of the screen, but can also resize elements or the space between elements proportionally according to the available space in the container and chosen parameters.

This is easy, needs no specific thought and it should not set it as a default for the programmer by panda, because you cannot know in advance what Frames are going to be used as.

Well, you can, if we allow the user to communicate this via the API.

It's not rocket science to implement it yourself, but it can get annoying if you have to do it every time and it seems reasonable to make this thing easier for the user. I do think it's out of scope for this issue, I just wanted to make sure we give some thought to this so we don't limit us from creating such a feature in the future.

I think the discussion of whether we actually want to create such a system in Panda should be left to a separate issue, to keep this one focused.

Because they require flexible textures, right now, textures are inherited from the scale of the frame, so if you rescale the frame, you also change the scaling the texture as well. Which means you "squish" the texture if you scale the frame on the x axis. Same for Frame size. If I'm wrong about this, I'd appreciate documentation on it :)

Yes, this deserves some consideration - achieving the result in these examples requires 9-slice scaling support and we should think about whether and how we should support that, since it's a fairly fundamental feature of UI theming.

@rdb
Copy link
Member Author

rdb commented Feb 10, 2024

Because they require flexible textures, right now, textures are inherited from the scale of the frame, so if you rescale the frame, you also change the scaling the texture as well. Which means you "squish" the texture if you scale the frame on the x axis. Same for Frame size. If I'm wrong about this, I'd appreciate documentation on it :)

I was digging a bit into PGui and it turns out this is supported, just undocumented:
https://discourse.panda3d.org/t/9-slice-scaling-with-directgui/29906

In PGui, all these things are part of the PGFrameStyle. I think the main thing to provide themeability is to provide an easy way to access, store and apply PGFrameStyle objects.

@ArsThaumaturgis
Copy link
Contributor

ArsThaumaturgis commented Feb 10, 2024

Regarding the health- and mana- bars shown, for myself I'd be inclined to use shaders for those: doing so provides more control over the appearance of the bar, and allows for some nice fancy effects like fluids sloshing and energy shields glowing, and so on.

The one exception is that the health-bar with the shaped end could, I imagine, be implemented simply via texture-offsetting. That is, instead of scaling the entire bar, just ensure that the texture is in "clamp" mode and then offset it by the appropriate amount, creating the impression of the bar filling and emptying.

~

Regarding things like button-events, I think that I prefer the current system of assigning a function to the object's callback-variable.

i.e.

button.command = myCallbackFunction

def myCallbackFunction( <... args ...> )
    <... code ...>

It's quick, it's easy, and it's (to me at least) nicely clear.

~

Regarding the matter of layout, I'm inclined to think that while layout objects are potentially useful, it's more important to have a visual tool in which to design one's UIs.

I daresay that placing things visually, having not only the ability to arrange objects by sight but also immediate visual feedback on layout, will in general tend to be far more intuitive than a code-based approach, even if that code-based approach is excellent.

(If DirectGui Designer could be adapted to serve PGui, that might do the job.)

Now, such a tool is likely beyond the scope of this thread, but I want to mention the above against the idea of spending too much energy on code-based layout elements.

@BMaxV
Copy link
Contributor

BMaxV commented Mar 11, 2024

There are no good ways to set default fonts for directgui. rdb suggested setDefaultFont in DGG, but that variable isn't used for buttons. TextProperties.setDefaultFont is supposed to work for textnodes but also has no effect.

This goes a bit into the styling problem that directgui wanted to solve a bit. I don't think the global is that bad in this case, but the question remains whether we want an object to store style info, pass it to constructors every time or solve it with a global like directgui.

@ArsThaumaturgis
Copy link
Contributor

Hmm... My main qualm with a singular "setDefaultFont" is that I may want multiple styles with multiple fonts.

For example, I might want a style that's used for headings, and one for text shown when the player collects an item, and another for paragraph-text. And the former two might share a font--but with different settings.

Now, if "setDefaultFont" were to take a style-reference, which could then be passed to DirectGUI widgets, then that might work...

... But then, there would likely be other style-elements to be applied. Now, this could be done via multiple such globals--but that starts to become awkward and error-prone.

So, I'd thus be inclined to prefer style-objects that could be passed wholesale to DirectGUI widgets.

@BMaxV
Copy link
Contributor

BMaxV commented Mar 17, 2024

In the effort of nice looking buttons, the following was found:

(Setting all this via text node works fine, it's part of my example, so I'm going to use that as a reference)


Direct can set text color this way

b = DirectButton(text=("OK", "click!", "rolling over", "disabled"),
                 scale=.5)
myfont = base.loader.loadFont('myfont.egg')
b['text_font'] = myfont
b['text_color'] = (1,0,0,1)

But it can not set the outline this way. I did not find the appropriate keywords, if they have been setup and it's not documented.


With PGUI things are weird.

  • You can load fonts and assign them to buttons
  • You can change the color
  • You can define an outline, but has to be either same color of the text, or black.

Let me explain/demonstrate. I think you should have both my demo file https://github.com/BMaxV/basic_panda3d_PGUI and something to execute it ready for this to make sense. And I put some comment markers in the limiting where I will change code.


  • if the button is setup, but the font is assigned later, nothing will happen. The font to be assigned to the textnode belonging to the button.
        # start of button color experimentation zone      
        
        font = self.b.loader.loadFont("Compyx-Solid.ttf")
        font.setPixelsPerUnit(100)
        
        red = (1,0.2,0.2,1)
        white = (1,1,1,1)
        black = (0,0,0,1)
        
        font.set_outline(white,0.5,0.5)
        font.set_fg(black)
        
        b.setup("hello text",0.1) # display text and the bevel width
        b2.setup("hunter2",0.5) 
        
        b1_text_node = b.getTextNode()
        b1_text_node.set_font(font)
        
        # end of button color experimentation zone.

  • if the font is setup first, it will affect the button, but it will affect BOTH buttons, even though the textnode was only grabbed from one, and it will respect the SHAPE of the outline but not the color
        # start of button color experimentation zone      
        
        font = self.b.loader.loadFont("Compyx-Solid.ttf")
        font.setPixelsPerUnit(100)
        
        red = (1,0.2,0.2,1)
        white = (1,1,1,1)
        black = (0,0,0,1)
        
        font.set_outline(white,0.5,0.5)
        font.set_fg(black)
        
        b.setup("hello text",0.1) # display text and the bevel width
        b2.setup("hunter2",0.5) 
        
        b1_text_node = b.getTextNode()
        b1_text_node.set_font(font)
        
        # end of button color experimentation zone.

  • if set_text_color is used before the setup step, it will change the color of the outline of the button text. but not the text color. And this is "per button" again and not global.
       # start of button color experimentation zone 
       
       font = self.b.loader.loadFont("Compyx-Solid.ttf")
       font.setPixelsPerUnit(100)
       
       red = (1,0.2,0.2,1)
       white = (1,1,1,1)
       black = (0,0,0,1)
       
       font.set_outline(white,0.5,0.1)
       font.set_fg(black)
       
       b1_text_node = b.getTextNode()
       b1_text_node.set_font(font)
               
       b2.setup("hunter2",0.5)
       b1_text_node.set_text_color(red)
       b.setup("hello text",0.1) # display text and the bevel width
                       
       # end of button color experimentation zone.
  • now that we are using set_text_color it will respect set_fg, which it does not do, otherwise. But ONLY if it's the same color as the outline. Otherwise you get black.
        # start of button color experimentation zone      
        
        font = self.b.loader.loadFont("Compyx-Solid.ttf")
        font.setPixelsPerUnit(100)
        
        blue = (0,0,1,1)
        red = (1,0.2,0.2,1)
        white = (1,1,1,1)
        black = (0,0,0,1)
        
        font.set_outline(white,0.5,0.1)
        font.set_fg(blue)
        
        b1_text_node = b.getTextNode()
        b1_text_node.set_font(font)
                
        b2.setup("hunter2",0.5)
        b1_text_node.set_text_color(red)
        b.setup("hello text",0.1) # display text and the bevel width
                        
        # end of button color experimentation zone.

@BMaxV
Copy link
Contributor

BMaxV commented Apr 5, 2024

The other day I made a button constructor for textnode and DirectFrame, where I would remove the DirectFrame and replace it with PFrame when we have figured out how the hover_in / hover_out is solved with PGUI.

Most importantly for that was that I wanted a unified style input where I can preload font, background etc. and just pass that around and have all buttons look nice.

The question now is what the save file format should look like, to make it even easier to for example, share configs. .css sounds ideal for me, we could interpret what we do as "classes" and everyone would immediately able able to apply their knowledge of how ccs works. And we can pick and choose which attributes to support and how.

I just wanted to bring it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issues opened primarilly to start a discussion enhancement
Projects
None yet
Development

No branches or pull requests

5 participants