-
Notifications
You must be signed in to change notification settings - Fork 4
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
Limit draw updates #202
Limit draw updates #202
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good on first glance. The alternative to implementing the update batching/debouncing in the simulation is to implement it in the playfield drawer
Line 192 in 49d372f
public void setDrawables(final List<Drawable> drawables) { |
There you could use swing timers to trigger draw updates (this would be a bit nicer than the manual thread spawning and joining here). But I don't know where the biggest performance bottlenecks actually are.
...va/de/unistuttgart/informatik/fius/icge/simulation/internal/playfield/StandardPlayfield.java
Outdated
Show resolved
Hide resolved
I chose to implement this here instead of in the playfield drawer since |
} catch (@SuppressWarnings("unused") final EntityNotOnFieldException e) { | ||
//Entity has been removed from the field while this loop was running. | ||
//Just don't draw it and ignore the exception. | ||
if (this.awaitingEntityDraw) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually prefer the fast exit form over nesting everything in an if. This makes the method easier to read, as all the fail conditions are at the top and can be ignored for the rest of the method body.
if (!this.awaitingEntityDraw) {
return; // no updates to draw
}
// rest of the method
...va/de/unistuttgart/informatik/fius/icge/simulation/internal/playfield/StandardPlayfield.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Fabian Bühler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now. (But I did not run the code myself, so merge at your own risk.)
I cannot merge any pull request as i am not a maintainer of this project |
It looks like I don't have the rights to make you a project maintainer. You should ensure that you have at least 1-2 active maintainers with the correct access rights (@neumantm should be able to help with that). If you want me to merge just leave me a thumbs up. (but I may not be available for emergency merges on a short notice) |
This change limits the between individual calls of
drawEntities()
. This waydrawEntities()
is not called every time a new Entity is created. This can reduce Lag when multible entities are added to the PlayingField at the same time. The time betweendrawEntities()
calls can be changed with thesetTimeBetweenDraws
method which accepts the time in milliseconds. The time is initially set as 32ms between draw calls, which equates to a maximum of 31.75 Draws/Second. This implements #194.