Layouting corner case?

I have a layouting corner case (i guess) that manages to crash the internal ui sizing.
I have no custom components used, all are stock lemur stuff.

Why do I post this? I guess that a bit earlier in that layouting (there where the problem starts) an handling is either missing, or that one of those calculations is wrong and hope that this might help to improve it.

I do not set any size anywhere myself, all is layouted via the prefered sizes.

java.lang.IllegalArgumentException: Size cannot be negative:(433.09885, -22.835289, 0.0)
	at com.simsilica.lemur.core.GuiControl.setSize(GuiControl.java:242)
	at com.simsilica.lemur.component.SpringGridLayout$Entry.setSize(SpringGridLayout.java:537)
	at com.simsilica.lemur.component.SpringGridLayout.reshape(SpringGridLayout.java:310)
	at com.simsilica.lemur.core.GuiControl.setSize(GuiControl.java:259)
	at com.simsilica.lemur.component.BorderLayout.reshape(BorderLayout.java:166)
	at com.simsilica.lemur.core.GuiControl.setSize(GuiControl.java:259)
	at com.simsilica.lemur.component.SpringGridLayout$Entry.setSize(SpringGridLayout.java:537)
	at com.simsilica.lemur.component.SpringGridLayout.reshape(SpringGridLayout.java:310)
	at com.simsilica.lemur.core.GuiControl.setSize(GuiControl.java:259)
	at com.simsilica.lemur.core.GuiControl.revalidate(GuiControl.java:351)
	at com.simsilica.lemur.core.GuiControl.controlUpdate(GuiControl.java:318)
	at com.jme3.scene.control.AbstractControl.update(AbstractControl.java:128)
	at com.jme3.scene.Spatial.runControlUpdate(Spatial.java:736)
	at com.jme3.scene.Spatial.updateLogicalState(Spatial.java:879)
	at com.jme3.scene.Node.updateLogicalState(Node.java:230)
	at com.jme3.scene.Node.updateLogicalState(Node.java:241)
	at com.jme3.app.SimpleApplication.update(SimpleApplication.java:243)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:151)
	at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:197)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:232)
	at java.lang.Thread.run(Thread.java:745)

The ui is structured like this, Simple table is an Grid with an ArrayGridModel, used to simplify building often required table semantics. At the end the parser is attached if that helps

<Container layout="BorderLayout">
	<Container constraints="North">
		<Button id="menuSave" text="Save"/>
		<Button id="menuLoad" text="Load"/>
		<TextField id="shipName"/>
		
		<Button id="pitchPlus" text = "P+"/>
		<Label id="pitch"/>
		<Button id="pitchMinus" text = "P-"/>
		<Button id="yawPlus" text = "Y+"/>
		<Label id="yaw"/>
		<Button id="yawMinus" text = "Y-"/>
		<Button id="rollPlus" text = "R+"/>
		<Label id="roll"/>
		<Button id="rollMinus" text = "R-"/>
	</Container>
	
	<Container constraints="West">
		<Label text="Filter" id="filter"/>
		<Button id = "up" text="up"/>
		<SimpleTable id="blockTreeTable">
			<Column id="blockName" title="blockName" renderer="Label" field="name" />
			<Column id="mass" title="mass" renderer="Label" field="mass" />
			<Column id="blockPreview" title="blockPreview" renderer="Image" field="blockPreview" />
			<Column id="info" title="info" renderer="Button" value="?" />		
		</SimpleTable>
		<Button id = "down" text="down"/>
		
		
		<Label id="cursorInfo"/>		
	</Container>
</Container>

I can give you a dump of the praser, if that helps, but it is not in a user friendly way currently :slight_smile:

Not even on the outer container?

That’s very strange. It would be interesting to know which GUI element is reporting its preferred size incorrectly because that’s the only way I could think this could happen. Else maybe the grid panel is doing something strange but it should always try to be as big as its biggest element * columns/rows.

We just need to track down which element is calculating its preferred size or layout incorrectly.

Yes, thats part of the problem. The other side would be to add exception handling to isolate further similar errors.
Eg if prefered size is negative ect. Any good ideas what should be within some values? I would create a pullrequest then that checks what i can to prevent similar further errors.

It’s not that preferred size is negative… it’s the the sizing pass ends up with some negative size.

First, from the bottom up, all of the components are asked what their preferred size is. Then from the top down, they are told what their size will be. Someone on the way up said they would be size X but on the way down they took size X + extra and now that extra size isn’t available to the other components when sizing… so someone eventually gets a negative size.

I don’t remember if I’d threaded trace logging through all of this or not.

So,

I should kinda check for objects that within one layouting tick ask for a smaller size than they are after resizing?

If you want to step through in the debugger, it’s GuiControl.getPreferredSize() that calculates the size all the way up… then GuiControl.setSize() that sets the size all the way down.

Someday I will add trace logging to these calls.

At the root, the panel with no Lemur parent, setSize(getPreferredSize()) is called for the outer sizing. (Unless you are overriding that with your own size.) And so if the getPreferredSize() is accurately calculated then setSize() should be fine.

And if you want more detail on what I’m talking about, you can read these docs:

…with pictures and everything.

I managed to follow that bug down to the Springlayout.

apparently I have one that has a size of 433,0,0 for some reason. (Which is pretty small but not invalid right?)
Reason for the small size seems to be the Borderlayout above it, (though if I only have the north panel, there is enough size for at least 100pixel plus left on screen)

It seems that
protected void distribute(float[] sizes, float[] prefs, float totalSize, float totalPref, FillMode fill, Axis axis) {
for( int i = 0; i < sizes.length; i++ ) {
sizes[i] = weighted(i, prefs[i], totalSize, totalPref, sizes.length, fill, axis);
}
}
can store negative numbers in sizes. If I understood your layouting correctly, it is not supposed to do this right? At minimum it should use 0 instead of a negative number?

Cause is in weigthed for the grow distribution

case Even:
// Even means that they all grow evenly… not
// that they are forced to be the same size. So
// we take the total difference and divide it evenly
// among the children.
return pref + (totalSize - totalPref)/count;

If I understand correctly, it does should never return <0, as the minimum additional logically can only be 0?

Or in other word’s I’m missing ~22 px there (thats the negative number i get).

So question is is it supposed to crash in such a case, or use a 0 size and try to continue?
I ask because my ui’s can be user created (in limited ways trough the xml from the shipcomputer) and this means
a) it is a bug, it needs to be fixed (and I can spend more time to debug it)
b) it is by design, and I somehow needs to account for this, and prevent users to input inalid layouting ui’s (if that is even possible) → would probably mean I would have to throw out everything except absolute layouting :confused:

Could it be that fill mode even is not causing the preferred size to be calculated as numCols * maxColSize?

I mean, we could simply Math.max(0, size) everywhere and display broken GUIs… but it’s a bug in the user code so I felt it would be better to indicate that. There really isn’t any guarantee that a particular component will be happy with a 0 size, to be honest.

Because remember the layout is just laying out children and they may have their own components that are gobbling up space also. So essentially we’d need to make every setSize() call or reshape() deal with clamping the size and then you’d lose the except/error condition completely.

It seems we’d trade “Why is this throwing an exception?” to “Lemur makes crappy looking UIs.”

It’s probably safer, though… but we’d have never found the bug in SpringGridLayout if we clamped, though.

You misunderstood, I don’t mean to clamp everywhere, but more validate my assumption that I understood right.

It would make way more sense to skip 0 size objects anyway (cullhint always and done) then to try to render anything in them.

What should be true… and apparently isn’t… is that it should always be safe to call someThing.setSize(someThing.getPreferredSize()).

So something there is broken and it may be SpringGridLayout.

The problem with simply leaving out size 0 things is that it won’t be just the GUI element itself but likely random sub-components in its stack. The most obvious ones might make sense to look at but the others not so much… and it gets worse as you nest further and further.

I’m not against clamping to >=0 but it concerns me that it might make the real bugs more subtle.

Ok so before I try to implement a system for the clamping, while still doing error reporting.

someThing.setSize(someThing.getPreferredSize()) is allowed
someThing.setSize(someThing.getPreferredSize()+anything) is also allowed

what about

someThing.setSize(someThing.getPreferredSize()-something) is it allowed? if not is it sensible to report an error and abort layouting (of this subtree) as soon as that situation happens?

Yes, those should always work and should not require clamping. If the layouts are behaving properly then those are both legal.

Currently, this is the case that may throw an error if the caller has asked the layout to do something that it cannot do. We can kind of decide if that’s the way we want it to behave.

Sometimes when one is developing a framework, they want to know about all of these errors. Sometimes users just want something to see without odd exceptions. Technically, users trump my desire here, really.

One other thing to consider, in all my years of developing UI layouts, swing wrappers, screen generators, etc… every time I tried to make things flexible to autolayout, most of my users preferred to specifically set the sizes of every thing. I find this to often be even MORE true in game development. Seems like it’s quite frequent that a screen would be mocked up in photoshop before code is ever written.

Dynamic resize and layout is something seen usually only in the more advanced “I’m implementing an OS in my game” sort of UIs. (Nothing wrong with that as some games require it but you know what I’m talking about.) It is heavily on my stack of touch stones that I not sacrifice too much simplicity to cover these cases. (Often those users will at best build custom stuff on Lemur’s base, anyway.)

Hm I guess a generic way is to much work, and as you say a bit out of scope.

However allowing a custom error Handler that catches the exceptions, and then skips the subtree updates should be possible without too much work.

This is also the solution I would prefer, it is probably never possible to account all cases, especially with user creates ui’s, but it should be possible to just catch them without crashing the rest of the game if required.

So my idea would be to extend the GUIGlobals to allow setting a Errorhandlier.
Adjust the layouting logic to be wrapped in a try catch, rethrow unmodified if no handler exist, else dispatch via handler and do not rethrow.

This would in my original case result in a brokenly layouted ui
But also a error in the errorhandler, allowing me to replace the ui with an message that the layouting of this screen is not possible, and that it was removed due to that, allowing the rest of the game to continue.

Seems this could be accomplished with a custom exception class and just calling setSize() yourself if you want to catch it, no?

I cannot follow, as I do not call setSize? it is called within the updateState.

But you could call it if you wanted to see if it would throw an exception.

AH i get what you mean now, so I could validate, if it will layout correctly, and also try to figure core issue better