Vertex loader cleanup

[degasus]

This branch was designed to move all vertex attributes information into VideoCommon. So now they are defined by the VertexLoader and the backends are supposed to just pass them through.

But as VideoSW also uses the VertexLoader in a very hacky way, we aren' t allowed to use any new feature now. So in the end, it's a cleanup and a feature preparation.

[degasus]

In which way do we want to fix the videosw issue?
a) use more of the common vertexloader, but so we'd have to convert everything as the videocommon vertexloader wants to and fix it afterwards
b) complete fork vertexloader. I've done the first step (for positions): http://pastie.org/8681971 -- Please don't see this patch as ready for merging, it's poc

[hrydgard]

In PPSSPP I created a "VertexReader" that reads OpenGL formatted vertex data (of any format). This lets me share the entire vertex loader with no issues except possibly slightly slowing down the software renderer. I'd do the same thing if I were back working on Dolphin.

[degasus]

hrydgard: yeah, I'm also thinking about forking our videosw based on the hardware api. So it likely will be a)

[delroth]

+1 for making our sw backend closer to the hw implementations when it can
be instead of having it be a (mostly) separate implem.

On Thu, Jan 30, 2014 at 3:14 PM, Markus Wick notifications@github.comwrote:

hrydgard: yeah, I'm also thinking about forking our videosw based on the
hardware api. So it likely will be a)

Reply to this email directly or view it on GitHubhttps://github.com//pull/11#issuecomment-33690830
.

Pierre "delroth" Bourdon delroth@gmail.com
Software Engineer @ Zürich, Switzerland
http://code.delroth.net/

[lioncash]

+1 for the same reason @delroth stated.

[neobrain]

Apart from that, looks fine to me. Will merge once the coding style issues and the lookup table are fixed. Will comment on the "merge or not merge?" issue later.

[degasus]

I'm fine with hard coding position, but only in videocommon. For consistensy, the backends should handle it like all other attributes.
btw, 2 or 3 elements position and us|float? So the offset won't matter ;)

[degasus]

Sorry, line based comments seem to be removed silencely on updates (push -f), but I've fixed the coding style + the lookup table. The other patches haven't changed.

[degasus]

Not solved comment from neobrain: "Generally: Why do we not want to hardcode the position? Seems unnatural not to have a position in a vertex format."

[neobrain]

I'd still prefer to have the if (format->enable) check removed for the position, possibly even with a comment that it's not possible to disable position (unless I'm completely mistaken here and this is actually possible on Flipper).

As it stands right now, when looking over the code, people unfamiliar with will ask themselves "huh, you can actually disable the position? what sense does this make?" and then have to look up that format->enable is always true anyway.

[degasus]

https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/CPMemory.h#L77

But I don't know what happens when this is set to 0 (not present).

imo unfamiliar people will ask we do we handle the position attribute in an other way than all others. I just don't want to hardcode it at different places.

[neobrain]

Yeah ok, looks like theoretically it should be possible to disable position, nevermind then.