webai-community / aquarium

BSD 3-Clause "New" or "Revised" License
26 stars 7 forks source link

Class & directory hierarchy for the ANGLE backend #166

Open hujiajie opened 4 years ago

hujiajie commented 4 years ago

This is the continuation of #165

  1. Does it deserve to introduce ContextANGLE inheriting from ContextGL?

  2. If yes to (1), where should ContextANGLE.{h,cpp} be placed?

  3. If yes to (1), how will the class and directory hierarchy look like if we want to add a native GLES backend in the future?

hujiajie commented 4 years ago

I'm not sold by the value of ContextANGLE. @gyagp @shaoboyan @Jiawei-Shao What's your opinion?

gyagp commented 4 years ago

My opinion:

  1. Yes
  2. create a folder named "angle" under aquarium-optimized, and put files there. BTW, I prefer the name "ContextAngle.h|cpp" as we already used "Dawn" instead of "DAWN".
  3. create folder named "opengles" under aquarium-optimized.
shaoboyan commented 4 years ago

https://github.com/webatintel/aquarium/issues/166#issuecomment-699792292 +1

Jiawei-Shao commented 4 years ago

I remember the ContextGL context is using desktop OpenGL API instead of the OpenGL ES one used by ANGLE, right? Will we plan to test something that is only supported in desktop OpenGL and not supported by ANGLE?

hujiajie commented 4 years ago

My opinion:

  1. Yes
  2. create a folder named "angle" under aquarium-optimized, and put files there. BTW, I prefer the name "ContextAngle.h|cpp" as we already used "Dawn" instead of "DAWN".

My impression is that ANGLE and Dawn are the most standard spelling.

  1. create folder named "opengles" under aquarium-optimized.

How will ContextANGLE, ContextGLES, ContextGL inherit from each other?

hujiajie commented 4 years ago

I remember the ContextGL context is using desktop OpenGL API instead of the OpenGL ES one used by ANGLE, right? Will we plan to test something that is only supported in desktop OpenGL and not supported by ANGLE?

We use the common set of desktop OpenGL and OpenGL ES, so that's basically OpenGL ES. No idea about modern desktop OpenGL features yet.

hujiajie commented 4 years ago

How will ContextANGLE, ContextGLES, ContextGL inherit from each other?

One proposal is:

ContextGL : public ContextGLBase {};

ContextGLES : public ContextGLBase {};

ContextANGLE : public ContextGLES {};

The directory layout is:

//src/aquarium-optimized/$opengl_base
//src/aquarium-optimized/opengl
//src/aquarium-optimized/opengles
//src/aquarium-optimized/angle
//src/aquarium-optimized/dawn
//src/aquarium-optimized/d3d12

-1 from my side, but how do you like it?

hujiajie commented 4 years ago

Or

//src/aquarium-optimized/*.{h,cpp}
//src/aquarium-optimized/opengl/*GL.{h,cpp}
//src/aquarium-optimized/opengl/desktop/*Desktop.{h,cpp}
//src/aquarium-optimized/opengl/es/*ES.{h,cpp}
//src/aquarium-optimized/opengl/es/angle/*ANGLE.{h,cpp}
//src/aquarium-optimized/dawn/*Dawn.{h,cpp}
//src/aquarium-optimized/d3d12/*D3D12.{h,cpp}

i.e., subclasses are implemented as subdirectories (assuming no multi-inheritance will happen). Naming of the files remain TBD.

hujiajie commented 4 years ago

IIUC, the latest proposal is (the final naming still TBD):

//src/aquarium-optimized/common
//src/aquarium-optimized/common/opengl
//src/aquarium-optimized/opengl
//src/aquarium-optimized/opengles
//src/aquarium-optimized/angle
//src/aquarium-optimized/dawn
//src/aquarium-optimized/d3d12

But I'm still a little confused in some details:

  1. What led to the preference of a flattened hierarchy than a tiered one? Sorry I forgot the main argument.

  2. It sounds common/opengl will be a quite heavy directory, compared with opengl, opengles and angle. Is this desired? Or do you prefer to hoist that directory, even it's not a concrete backend?

  3. What's the final decision for the role of //src/aquarium-optimized/opengles? If //src/aquarium-optimized/opengles works as the common part between system GLES and ANGLE backend, and //src/aquarium-optimized/angle will load the ANGLE implementation of GLES, will there be a directory like //src/aquarium-optimized/opengles_system to load the system-wide library? If //src/aquarium-optimized/opengles means exclusively the system GLES backend, then //src/aquarium-optimized/angle should no longer inherit from it, right? How to handle the common routines like EGL initialization then?

@gyagp @shaoboyan @Jiawei-Shao @qjia7