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

Oh No! More shader changes! #2476

Merged
merged 9 commits into from Feb 4, 2017

Conversation

Projects
None yet
2 participants
@yuriks
Member

yuriks commented Jan 27, 2017

Oh No! More Lemmings!

These include small functional changes, so not strictly refactors, but nothing major.

Again, recommend reviewing each commit individually.

INSERT_PADDING_WORDS(4);
/// Number of input attributes to the vertex shader minus 1
BitField<0, 4, u32> max_input_attrib_index;

This comment has been minimized.

@Subv

Subv Jan 27, 2017

Member

Does this change break/fix anything obvious?

@Subv

Subv Jan 27, 2017

Member

Does this change break/fix anything obvious?

This comment has been minimized.

@yuriks

yuriks Jan 27, 2017

Member

Not really, for correct functioning (under the current VS-only setup) these two have to be set to the same value, this is just for better isolation between the funcional units of the hardware. Ths is the actual configuration of the vetex frontend, whereas the other one was fr the shader input (which the frontend feeds to). Afaik for GS support the previous code would need to switch between the correct input unit, whereas with this reg this will always have the appropriate setting.

@yuriks

yuriks Jan 27, 2017

Member

Not really, for correct functioning (under the current VS-only setup) these two have to be set to the same value, this is just for better isolation between the funcional units of the hardware. Ths is the actual configuration of the vetex frontend, whereas the other one was fr the shader input (which the frontend feeds to). Afaik for GS support the previous code would need to switch between the correct input unit, whereas with this reg this will always have the appropriate setting.

Show outdated Hide outdated src/video_core/shader/shader.cpp
OutputVertex ret{};
std::array<float24, 24> vertex_slots;
};
static_assert(sizeof(vertex_slots) <= sizeof(ret), "Struct and array have different sizes.");

This comment has been minimized.

@Subv

Subv Jan 27, 2017

Member

Why <= instead of == ?

@Subv

Subv Jan 27, 2017

Member

Why <= instead of == ?

This comment has been minimized.

@yuriks

yuriks Jan 27, 2017

Member

OutputVertex has additional members and padding which are used internally. (I do remove those on the next commit.)

@yuriks

yuriks Jan 27, 2017

Member

OutputVertex has additional members and padding which are used internally. (I do remove those on the next commit.)

@Subv

Subv approved these changes Jan 27, 2017

LGTM

@yuriks

This comment has been minimized.

Show comment
Hide comment
@yuriks

yuriks Jan 28, 2017

Member

Updated with a commit to fix the build error on Linux.

Member

yuriks commented Jan 28, 2017

Updated with a commit to fix the build error on Linux.

yuriks added some commits Dec 18, 2016

VideoCore: Change misleading register names
A few registers had names such as "count" or "number" when they actually
contained the maximum (that is, count - 1). This can easily lead to hard
to notice off by one errors.
VideoCore/Shader: Clean up OutputVertex::FromAttributeBuffer
This also fixes a long-standing but neverthless harmless memory
corruption bug, whech the padding of the OutputVertex struct would get
corrupted by unused attributes.
@yuriks

This comment has been minimized.

Show comment
Hide comment
@yuriks

yuriks Jan 30, 2017

Member

Rebased/autosquashed

Member

yuriks commented Jan 30, 2017

Rebased/autosquashed

@yuriks

This comment has been minimized.

Show comment
Hide comment
@yuriks

yuriks Feb 3, 2017

Member

I plan on merging this sometime this weekend.

Member

yuriks commented Feb 3, 2017

I plan on merging this sometime this weekend.

@yuriks yuriks merged commit 97e06b0 into citra-emu:master Feb 4, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yuriks yuriks deleted the yuriks:shader-refactor3 branch Feb 9, 2017

jfmherokiller added a commit to jfmherokiller/citra that referenced this pull request Mar 29, 2017

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