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

Re-enable Mixed particle rendering #749

Merged
merged 1 commit into from Oct 27, 2014

Conversation

Projects
None yet
3 participants
@lukaspj
Contributor

lukaspj commented Aug 6, 2014

There seems to have been some copy-pasta and maintenance issues that have caused Mixed rendering to be unfunctional.
This commit fixes those issues and re-enables mixed rendering, as explained here.

Currently renders the low-res particles at 1/4 the resolution. To increase or decrease this, look at line 77 in RenderParticleMgr.

Performance difference with 2 emitters, each emitting 1000 particles per second:

Method MSPF Impact
Highres 27 8.5
MixedRes 20 1.5
No emitters 18.5
@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Aug 6, 2014

Contributor

Oh btw, this could greatly benefit from being able to choose between different Offscreen resolutions.. Haven't quite fixed that one up yet.
1/4 seems to be really blurry but give great performance improvements, 1/2 is a good middle way and 3/4 would be good for some performance improvements with negligible visual impact .

Contributor

lukaspj commented Aug 6, 2014

Oh btw, this could greatly benefit from being able to choose between different Offscreen resolutions.. Haven't quite fixed that one up yet.
1/4 seems to be really blurry but give great performance improvements, 1/2 is a good middle way and 3/4 would be good for some performance improvements with negligible visual impact .

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Aug 7, 2014

Contributor

New benchmarks, with images showing the quality of the render:

Resolution MSPF Impact
100% 28 10
75% 27 9
50% 23.7 5.7
25% 21 3
No emitters 18

I propose that we make 50% the standard resolution, seems to be the best tradeoff between quality and speed.

Contributor

lukaspj commented Aug 7, 2014

New benchmarks, with images showing the quality of the render:

Resolution MSPF Impact
100% 28 10
75% 27 9
50% 23.7 5.7
25% 21 3
No emitters 18

I propose that we make 50% the standard resolution, seems to be the best tradeoff between quality and speed.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Aug 7, 2014

Contributor

Sounds good. When this gets accepted, if it does before you manage to add a way to change the proportion, we'll add a new issue for that. And maybe assign it to you if we're feeling cheeky ;P.

Contributor

crabmusket commented Aug 7, 2014

Sounds good. When this gets accepted, if it does before you manage to add a way to change the proportion, we'll add a new issue for that. And maybe assign it to you if we're feeling cheeky ;P.

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Aug 7, 2014

Contributor

Now uses 50% resolution, and I also edited the scripts such that EdgeDetectPostEffect is always enabled (since the MixedParticleRendering uses this).
If it is not enabled it should revert to highres rendering and throw an assert.
Edit: btw feel free to assign me to that, I want to figure out those rendertargets anyways :P

Contributor

lukaspj commented Aug 7, 2014

Now uses 50% resolution, and I also edited the scripts such that EdgeDetectPostEffect is always enabled (since the MixedParticleRendering uses this).
If it is not enabled it should revert to highres rendering and throw an assert.
Edit: btw feel free to assign me to that, I want to figure out those rendertargets anyways :P

@crabmusket crabmusket added this to the 3.7 milestone Oct 11, 2014

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 14, 2014

Contributor

Is there a way to turn this off, for example as a user preference? I think a console error is probably more appropriate than an assert. Actually, what does AssertWarn do?

Contributor

crabmusket commented Oct 14, 2014

Is there a way to turn this off, for example as a user preference? I think a console error is probably more appropriate than an assert. Actually, what does AssertWarn do?

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Oct 14, 2014

Contributor

Heh maybe I didn't explain properly.. This isn't a new feature, it's a bug fix for an old feature. Just set highresonly on particle datablocks to true to turn it off.

Contributor

lukaspj commented Oct 14, 2014

Heh maybe I didn't explain properly.. This isn't a new feature, it's a bug fix for an old feature. Just set highresonly on particle datablocks to true to turn it off.

@crabmusket crabmusket added Defect and removed New feature labels Oct 14, 2014

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 14, 2014

Contributor

Oh I understood that. Probably mislabeled it. Glad there's a way to turn it off, if that's what people want. Does this play nice with Basic Lighting?

Contributor

crabmusket commented Oct 14, 2014

Oh I understood that. Probably mislabeled it. Glad there's a way to turn it off, if that's what people want. Does this play nice with Basic Lighting?

@lukaspj

This comment has been minimized.

Show comment
Hide comment
@lukaspj

lukaspj Oct 14, 2014

Contributor

I don't see why it wouldn't but haven't tested it.. I'll test it tomorrow when I'm home from London.

Contributor

lukaspj commented Oct 14, 2014

I don't see why it wouldn't but haven't tested it.. I'll test it tomorrow when I'm home from London.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 14, 2014

Contributor

Thanks, mate!

Contributor

crabmusket commented Oct 14, 2014

Thanks, mate!

@crabmusket crabmusket self-assigned this Oct 27, 2014

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 27, 2014

Contributor

Confirmed appears to be working fine in basic.

Contributor

crabmusket commented Oct 27, 2014

Confirmed appears to be working fine in basic.

crabmusket added a commit that referenced this pull request Oct 27, 2014

@crabmusket crabmusket merged commit e02e542 into GarageGames:development Oct 27, 2014

1 check passed

default Merged build finished.
Details
@LuisAntonRebollo

This comment has been minimized.

Show comment
Hide comment
@LuisAntonRebollo

LuisAntonRebollo Nov 7, 2014

Contributor

warning C4553: '==' : operator has no effect; did you intend '='?

warning C4553: '==' : operator has no effect; did you intend '='?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment