ptitSeb / Eldritch

Eldritch port to the OpenPandora (meaning Arm / GLES2 / OpenAL): Status Working (with latest data file from Steam)
https://boards.openpandora.org/topic/18323-eldritch/
Other
3 stars 2 forks source link

AmigaOS4 (or general): when build with VBO support (by disabling NO_VBO) the person in the Mirror didn't moves #9

Open kas1e opened 3 years ago

kas1e commented 3 years ago

So far was able to reuse glMapBufferOES and glUnmapBufferOES Daniel add a year ago, and so VBO work (make the game be faster 2 times probably, which sometimes a bit slow still, but that another story), but find out that the person in the mirror didn't rotate. Maybe some other big-endian swaps need to be done when we use VBO ? When I go NO_VBO (i.e. our default path for now), the person in the mirror rotates well.

ptitSeb commented 3 years ago

Mmmm, the VBO/NO_VBO code is very very similar, with the the VBO part using glMapBuffer instead of plain arrays on pseudo VBO "0".

Basically, the code works with using a "Lock" on a buffer using void* const pData = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY); then the data is updated in pData then the buffer is "Unlock" with const GLboolean Success = glUnmapBuffer(GL_ARRAY_BUFFER); (of course the correct buffer is binded first). I don't any reason why the main character would stop moving with that change.

kas1e commented 3 years ago

Maybe that related to some of my changes: Daniel add those glMapBufferOES and glUnmapBufferOES as pure functions, not as extensions, so I had to do together with disabling NO_VBO in gl2.h, but in gl2vertexbuffer.cpp that:

Changing at the top file that part :

#ifdef HAVE_GLES
extern "C" {
//  void* eglGetProcAddress(const char*); // cannot include EGL/egl.h, as it conflict with other headers...
  void* SDL_GL_GetProcAddress(const char* proc);
}
#define eglGetProcAddress(aa) SDL_GL_GetProcAddress(aa)
static int MapBufferInited = 0;

#ifdef __amigaos4__
#define glMapBuffer glMapBufferOES
#define glUnmapBuffer glUnmapBufferOES
#else
static PFNGLMAPBUFFEROESPROC glMapBuffer = NULL;
static PFNGLUNMAPBUFFEROESPROC glUnmapBuffer = NULL;
#endif
#ifndef __amigaos4__
#define GL_WRITE_ONLY GL_WRITE_ONLY_OES
#endif
#endif

And at the end of GL2VertexBuffer::GL2VertexBuffer() change it like:

#if !defined(NO_VBO) && defined(HAVE_GLES)
if (!MapBufferInited) {
#ifndef __amigaos4__
    glMapBuffer = (PFNGLMAPBUFFEROESPROC)eglGetProcAddress("glMapBufferOES");
    glUnmapBuffer = (PFNGLUNMAPBUFFEROESPROC)eglGetProcAddress("glUnmapBufferOES");
#endif
    MapBufferInited = 1;
  }
#endif
}

Is it correct imho?

kas1e commented 3 years ago

Btw, by the main character, I mean in the mirror, not the whole person. I.e. I can move in-game of course, all fine, just when I come to the mirror, press "f", then in the mirror character didn't moves (while I can change clothes, etc). That was the same before while you didn't add more byte swaps even for NON_VBO path.

ptitSeb commented 3 years ago

Maybe that related to some of my changes: Daniel add those glMapBufferOES and glUnmapBufferOES as pure functions, not as extensions

Why?

Is it correct imho?

Yes, looks good. I'll integrate in the code later

Btw, by the main character, I mean in the mirror, not the whole person. I.e. I can move in-game of course, all fine, just when I come to the mirror, press "f", then in the mirror character didn't moves (while I can change clothes, etc). That was the same before while you didn't add more byte swaps even for NON_VBO path.

Yes, I understood.

kas1e commented 3 years ago

Why?

It's just we have only one single driver on amigaos4, and extensions as it make no sense for us. He adds the "fake" get_extension function, just for easy porting, but didn't add those OES functions to the list, so I had to use them as pure functions. He may add it later for easy porting, but not a big deal.

Strange why the usage of them makes an impact on the movements in the mirror...

ptitSeb commented 3 years ago

Why?

It's just we have only one single driver on amigaos4, and extensions as it make no sense for us. He adds the "fake" get_extension function, just for easy porting, but didn't add those OES functions to the list, so I had to use them as pure functions. He may add it later for easy porting, but not a big deal.

Meh, ok, but having the OES in place would make it easier to to compile and port stuff. Bah, it's not a big deal anyway.

Strange why the usage of them makes an impact on the movements in the mirror...

I assume it's not just the movement in the mirror, but all bone animations from any monster you may encounter.

kas1e commented 3 years ago

I assume it's not just the movement in the mirror, but all bone animations from any monster you may encounter.

Hm, will check now

kas1e commented 3 years ago

Yeah, you are right. All the bone animations from any monster the same non-working now :) I rechecked twice: with no_vbo path they work, with no_vbo path they didn't.

ptitSeb commented 3 years ago

You should check that with Daniel...

kas1e commented 3 years ago

Oh. If I only know how to explain to him those things. I mean, those new functions should or work or not work? But not like it render everything as expected, just animation in some parts (exactly in the monsters and mirror) didn't work. It sounds too much like the game's code?

Or it can be that something bad in the realization of those functions causes such issues?

I also noticed that we have quite more pandora/amigaos4 ifdefs, curious if those ones may have an impact or how the VBO path work...

kas1e commented 3 years ago

For example, see in gl2vertexbuffer.cpp, in the LOCK(), we have some amigaos4 ifdef taken place only when we in NO_VBO route (some m_ColorsPtr = m_ColorsVBO;). I do not remember why we do it (maybe it didn't necessary for now ?) Maybe some values just not byte-swapped still?

It just strange as we before have the same issue, but it went after byte-swapping. Maybe there just we miss some bit as well? Just new one happens only for the VBO route (at least I see their different structures used)

Or at the end of the gl2vertexbuffer.cpp , we have GLuint GL2VertexBuffer::InternalGetVBO(EVertexElements VertexType) { for VBO route, maybe there also need something like:

#ifdef __amigaos4__
      m_ColorsPtr = m_ColorsVBO;
#endif

Which do we do for the NON_VBO route?

kas1e commented 3 years ago

Or for example, in the same file, there is amigaos4 ifdef, which is not guarded by VBO/ NO_VBO: void GL2VertexBuffer:: Unlock(EVertexElements VertexType) {, maybe something of that sort flying somewhere.

ptitSeb commented 3 years ago

For example, see in gl2vertexbuffer.cpp, in the LOCK(), we have some amigaos4 ifdef taken place only when we in NO_VBO route (some m_ColorsPtr = m_ColorsVBO;). I do not remember why we do it (maybe it didn't necessary for now ?) Maybe some values just not byte-swapped still?

It just strange as we before have the same issue, but it went after byte-swapping. Maybe there just we miss some bit as well? Just new one happens only for the VBO route (at least I see their different structures used)

Or at the end of the gl2vertexbuffer.cpp , we have GLuint GL2VertexBuffer::InternalGetVBO(EVertexElements VertexType) { for VBO route, maybe there also need something like:

#ifdef __amigaos4__
      m_ColorsPtr = m_ColorsVBO;
#endif

Which do we do for the NON_VBO route?

That was an attempt to fix color issue. It's active on both with and without VBO. I'll remove it, it seems useless.

Oh. If I only know how to explain to him those things. I mean, those new functions should or work or not work? But not like it render everything as expected, just animation in some parts (exactly in the monsters and mirror) didn't work. It sounds too much like the game's code?

I think the issue is that the 2nd time the data are update, the data are not taken into account?

Something like

void* pData = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY);
/* fill in pData ... */
glUnmapBuffer(GL_ARRAY_BUFFER);
/* another frame later ... */
void* pData = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY);
/* fill in pData ... */
glUnmapBuffer(GL_ARRAY_BUFFER);
/* change seems ignored */
kas1e commented 3 years ago

Aha ok, at least something I can bring on Daniel :) Thanks!

kas1e commented 2 years ago

Interestingly i tried today a latest eldrith code , and while it builds fine and works as it, when i enable VBO by changing few GL2/gl2.h, GL2/gl2rendertarget.cpp and GL2/gl2rendertarget.h (by simple change amigaos4 on asfasdsdf) , i just have heavy crashes on running coming from ParticlesRenders->VertexBufferLcok_VertexBuffer_VertexElements.

For sake of tests i rollback to old versions of ogles2.library (just to be sure that not because of it), and it still crash and on old version of ogles2.library, while before it was not.

Can it be that some new changes (latest ones) when we merge with latest code, broke the VBO build ?