slow?

see…
http://www.user.shentel.net/pmurphy/Strategos/Strategos.vmod

I decided, before I go too much farther, to try to put together a scenario file similar to what would be in the game and see how it works. So the details are not all there yet but there is a number of counters representative of perhaps how many might be in a small-ish scenario.

problem…

Moving pieces seems a bit of a time lag. Since I have played enough larger games without this problem I am sure it is related to my module design somehow.

Is this going to bite me as the price to pay for using layers in the counters? Are the counters just too hi res? The custom zoon factors? Not sure what is the likely culprit.

Hi,

I have done some initial investigation, and it is a Vassal problem to do with the handling of the visible shape of counters containing Layer and Text Label traits. Counters having multiple Text Labels (like yours) suffer particularly. I believe this is a problem with the Combat Commander.

Joel, the problem is being caused by the use of our old friend java.awt.Area in the getShape() handling, especially in Labeler and Embelishment. Counters with 6 text labels like the Strategos module just die.

I’ve opened a new Sourceforge bug for it.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

That’s unexpected, since the To Be King module has a huge number of
text labels and seems to be ok.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hmm… Ah, it only happens if labelKey == null (ie no Key command). In Strategos, all of the Text Labels are displaying Dyanmic Property values. What about To Be King?

I don’t really understand what it is doing here. I think the whole getShape() call usage has to be reviewed, especially the usage of the Area class. Often is is called as getShape().getBounds(). boundingBox() would presumably be far more efficient? Assuming they are referring to the same thing of course :slight_smile:

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Last build of TBK I worked on removed the text labels from (most of) the
pieces - performance way up as a result

CC does not use text labels, but the counters can comprise of up to ~15-20
layers and there is a lot of DP/ layer relations, so there still may be
something to what you say as it relates to Embellishment, Text labels and no
key commands that DP interact

Post generated using Mail2Forum (mail2forum.com)

Joel,

I’ve done a bit more investigation on this, here are some ideas.

The problem is definitely caused by the use of the Area class in the implementation of GamePiece.getShape() in various counters. Creating new Area objects is especially expensive.

getShape() is called repeatedly in the screen redisplay code, often merely to get its bounds (i.e. piece.getShape().getBounds()). In these cases, a call to boundingBox() should be much more efficient.

My understanding is that getShape() should return the Shape of the counter that responds to selection clicks. When a counter is highlighted, then getShape() gives you that shape that should be highlighted by any Selection highlighters. This may or may not include graphic content produced by a given trait.

On the other hand, boundingBox() should return a Rectangle that includes all grahpical output of the counter.

The Shape of the counter is currently built up from the Union of the following (and only the following):

  • The bounds of an image defined in the BasicPiece
  • The bounds of the currently active image in a Layer
  • The shape of the rotated image in a Can Rotate
  • The bounds of a Text Label that does not have a Key Command
  • The shape of a Non-rectangular image
  • The shape if Obscured (An Image or obscured view)

Any module that uses any of these particular facilities can run into performance problems. BasicPiece and Non-rectangular are not a performance issue, the shape is pre-computed and cached.

I did some research looking for a drop-in replacement of Area() that has better performance, but couldn’t find anything.

I think the way ahead is to review all usage of Area() and to optimise getShape() and boundingBox(). I can see


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Oops, wasn’t quite ready to send that.

Anyway, leave this one with me. You can see where I am going. With a bit of caching and sensible optimization, I was able to remove the need to create almost all new Area()'s in Labeller and speeded up the Strategos module by 2 orders of magnitude.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

wohoo!

Joel,

swampwallaby-work@4287-4288 has optimization and caching of Area object production in Labeller, Embellishment and Obscurable. Labeller and Embellishment where the culprits in the Strategos module and it has changed from completely unplayable to quite responsive.

The result was so good, I haven’t looked deeper into the underlying structure of the getShape() chain.

Tim, I am very interested to see how Combat Commander responds to this change, but I could not get it running.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Why are you using clone() on the Areas? That’s a shallow copy, and so any
if Area you clone has any members which are Objects, the cloned Area will
share those members with the original. I’m almost sure this is not what
we want.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hi, I am not sure if this is the same issue or not… but I have opened my work-in-progress module with beta4, just to check if it still works (as I do with every new release), and I am getting various error messages, where I was getting none up to beta3 included. Since a counter which is made of just a bunch of text labels is not showing at all, I suspect it to be the culprit, or at least part of the problem.

BTW, yes… when I have too many text labels in this counter then everything becomes very slow. Definitively so with 3.0. Probably very much faster with 3.1 up to beta3, at least, though.

PS: is it possible to have multiple beta versions installed at the same time? It seems to refuse installation of beta4, say, if another version is still there, thus making it harder to compare versions.

Thus spake “stine020”:

Yes. On any platform besides Windows, just put whatever version wherever
you want. On Windows, using the installer, a Standard install will remove
older versions of VASSAL. In order to choose which older versions to keep,
pick a Custom install.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hmm, the mod works for me in Beta 4. Do you have a version of vassal with the changes I can grab and try to give it a spin in. Maybe the mod doesn’t like something or zip got corrupt

Thus spake “Brent Easton”:

In fact, now I’m sure that there’s no advantage in using Area.clone(), since
this is what the source looks like:

public Object clone() {
return new Area(this);
}


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Ha ha!

Ok, you can replace the x.clone() calls with a new Area(x).

Actually, I’ll do it in a moment as there is another optimization we can do in NonRectangular. It efficiently caches the generated Shapes, but uses GeneralPath’s. In every case, this is going to be converted into an Area() to be added to the BasicPiece getShape(), so we should store the NonRectangular Shape as a pre-converted Area instead.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Done,

swampwallaby-work@4295 gives you the update. 4287-4295 gives you the lot.

Cheers,
Brent.

*********** REPLY SEPARATOR ***********

On 21/10/2008 at 9:52 AM Brent Easton wrote:


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Joel,

I introduced a small bug, you will need to merge from 4298 also.

B.

*********** REPLY SEPARATOR ***********

On 21/10/2008 at 10:02 AM Brent Easton wrote:


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Hi Tim,

give this a go:

home.exetel.com.au/swampwallaby/ … indows.exe

Brent.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

That seems wrong, because now you’ll never enter the if block immediately
after this change, as path can’t be null now.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Duh! I new I should have gone to bed an hour ago.

Fixed in 4299 & 4300. The if will always be true now.

I actually tested it this time :slight_smile:

bed time…

*********** REPLY SEPARATOR ***********

On 21/10/2008 at 2:53 PM Joel Uckelman wrote:


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)