Add an error message for crashes without required UV sets
ClosedPublic
Actions

Authored by vladislavbelov on May 6 2017, 4:19 PM.

Details

Reviewers

wraitii
Stan

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP22037: Add an error message for crashes without required UV sets.

Trac Tickets

#4383

Summary

The 0AD crashes without any information, if artist selects the player_trans_ao material, but a model doesn't have a UV1 set (the model should be in a view).

It happens, because there is no checking that texcoordpointers were really setted. We should log at least an error message for artists.

For example #3638.

Test Plan

Run atlas
Add to preview any model which hasn't UV1 set, but should (change material from basic to ao)

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vladislavbelov created this revision.May 6 2017, 4:19 PM

elexis edited the summary of this revision. (Show Details)May 6 2017, 4:47 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1036/ for more details.

Harbormaster completed remote builds in B1488: Diff 1687.May 6 2017, 5:08 PM

Sandarac added a subscriber: Sandarac.May 6 2017, 8:43 PM

Sandarac added inline comments.

source/renderer/InstancingModelRenderer.cpp
1 ↗	(On Diff #1687)	Bump year I guess.

Stan requested changes to this revision.Aug 8 2017, 11:22 AM

Stan added inline comments.

source/renderer/InstancingModelRenderer.cpp
348 ↗	(On Diff #1687)	Maybe the check for stream flags could surround both blocks, as there should be no need to check it twice.
353 ↗	(On Diff #1687)	I wonder if there couldn't be a generic function for that as it's basically the same code with a few variants.

This revision now requires changes to proceed.Aug 8 2017, 11:22 AM

vladislavbelov added inline comments.Aug 8 2017, 8:24 PM

source/renderer/InstancingModelRenderer.cpp
348 ↗	(On Diff #1687)	It could be done with `for` and `define`, as it's the important place (performance), I'd prefer to use `define`, because I'm not sure, that a compiler will unroll the loop.
353 ↗	(On Diff #1687)	I'll add a way with define, it should complete your idea.

Adds the @Stan suggestion, with thoughts that the compiler could unroll the loop.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1836/ for more details.

Harbormaster completed remote builds in B2758: Diff 3074.Aug 10 2017, 11:24 PM

Stan accepted this revision.Aug 12 2017, 7:41 PM

This revision is now accepted and ready to land.Aug 12 2017, 7:41 PM

vladislavbelov mentioned this in D448: Added multiples UVs to animated model.Oct 28 2017, 8:42 PM

@wraitii Bump

Safe enough changes. Bump year again and you're good to go :p

Rebase the diff and cleanup includes.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/874/

Harbormaster completed remote builds in B6649: Diff 7189.Jan 2 2019, 3:23 PM

Stan accepted this revision.Jan 2 2019, 9:55 PM