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

Codechange: Sprite sorting optimization #7484

Closed
wants to merge 2 commits into from

Conversation

jmakovicka
Copy link
Contributor

@jmakovicka jmakovicka commented Apr 7, 2019

Another take on the sprite sorter optimization.

On top of the reverted 25ab9c1 ,
this patch adds the psdMax variable tracking the start of the
still intact pre-sorted portion of the parent sprite vector,
where the early bailout logic still works.

@jmakovicka
Copy link
Contributor Author

@jmakovicka jmakovicka commented Apr 7, 2019

This version of the 4k unzoom fix should avoid the sprite flickering caused by wrong sort order with the previous attempt, while maintaining the performance improvement.

src/viewport.cpp Outdated Show resolved Hide resolved
@jmakovicka jmakovicka force-pushed the master branch 2 times, most recently from ee5f1d8 to a5fdd2f Compare Apr 7, 2019
src/viewport.cpp Outdated Show resolved Hide resolved
src/viewport.cpp Outdated Show resolved Hide resolved
src/viewport_sprite_sorter_sse4.cpp Outdated Show resolved Hide resolved
src/viewport_sprite_sorter_sse4.cpp Outdated Show resolved Hide resolved
Another take on the sprite sorter optimization.

On top of the reverted 25ab9c1 ,
this patch adds the psdMax variable tracking the start of the
still intact pre-sorted portion of the parent sprite vector,
where the early bailout logic still works.
@PeterN
Copy link
Member

@PeterN PeterN commented Apr 8, 2019

This is still broken with regards to sorting, e.g. FIRS industry set.

print an exclamation mark if the output differs. input is presorted
for the original version too, as the exact result depends on the input.
@jmakovicka
Copy link
Contributor Author

@jmakovicka jmakovicka commented Apr 8, 2019

This is weird. This time, I used jmakovicka@c96cc5a to test that the two versions give the same result, at least if the original is suipplied with the same pre-sorted input, as the exact sort result is input dependent.

@PeterN
Copy link
Member

@PeterN PeterN commented Apr 8, 2019

The pre-sorting in your regression test messes up the sorting for the current sorter as well.

@michicc
Copy link
Member

@michicc michicc commented Apr 9, 2019

Without having actually checked anything: Should this maybe use a stable sort (i.e. std::stable_sort)?

@stale
Copy link

@stale stale bot commented May 9, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label May 9, 2019
@stale stale bot closed this May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants