Skip to content
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

Android GSoC 2021 #2261

Merged
merged 53 commits into from
Aug 30, 2021
Merged

Android GSoC 2021 #2261

merged 53 commits into from
Aug 30, 2021

Conversation

diabl0-NEMESIS
Copy link
Contributor

@diabl0-NEMESIS diabl0-NEMESIS commented Aug 18, 2021

Final Report: Android Support for ENIGMA (GSoC 2021)

ORGANIZATION: The ENIGMA Team

Mentors:

My GSoC project (#1880) included mainly 3 tasks:

  • Shader Fix
  • Adding a new audio system
  • Adding the tilt feature

    This pull request includes all of my commits, including the tasks mentioned above and some bug fixes.

TASK 1: SHADER FIX

In task 1 I started with some bug fixes like Refactoring for GLES build
which included:

  • moving functions from multiple files of OpenGL-Common to files with the same name in OpenGL-Desktop
  • creating GLES compatible versions of some OpenGL-Common functions in OpenGLES


This fix was needed because OpenGL-Common has shared code between GLES and all OpenGL and some functions were only available in OpenGL, not GLES because of which they were giving errors.


After this the next bug I fixed was the Scaling issue and presence of garbage in the android build which included :

  • fixing the Scaling issue for all the scaling options(fixed scale, keep aspect ratio, full scale)
  • removing graphical noise that is present when we open the game


For fixing the scaling issue I had to modify few functions such as window_get_width() & window_get_height() with the idea of using SDL_GL_GetDrawableSize() instead of SDL_GetWindowSize().
Since resizing window is not possible in android I modified initGameWindow() as well by removing the macro since we do not need it anymore.


For fixing the garbage issue I added some new functions like graphics_remove_garbage() in Platforms\SDL\event.cpp and d3d_enable_scissor_test() in OpenGL-Common\d3d.cpp.

static void graphics_remove_garbage(float x, float y, float width, float height) {
  graphics_set_viewport(x,y,width,height);
  enigma_user::d3d_enable_scissor_test(false);
  enigma_user::draw_clear(enigma_user::window_get_color());
}
void d3d_enable_scissor_test(bool enable) {
  if(enable==true)
     glEnable(GL_SCISSOR_TEST);
  else
     glDisable(GL_SCISSOR_TEST);
}


I modified the windowsResized() function in event.cpp used the graphics_remove_garbage() function inside it and the d3d_enable_scissor_test() was simply used to enable and disable the scissor test.

A scissor test is what tells the GPU to stay within a particular rectangle when filling. It's an optimization (which may or may not matter when already specifying a viewport). Once enabled, pixels outside the scissor box(drawing area) will be discarded, removing the garbage around our game.

Below is the comparison of what it looked like before and after adding these changes and building the game again:

game screenshots:

BEFORE AFTER

Both the scaling and garbage issues were fixed.


After this, I started my first task for which I created a sample game with some sprites and circles and added different colors to them just to see the error in our shader.
Below is the difference between Android and Windows before the shader fix :

Windows:

Android:

After this, my mentors suggested that I should learn GLSL so I learned few concepts of GLSL like swizzling, samplers, etc and I got to know that we can access all the 4 components of a vec4 in any order you want.

My fix for the Shader issue was using "TexColor = texture( en_TexSampler, v_TextureCoord.st ) * v_Color;" for OpenGL and "TexColor.bgra = texture( en_TexSampler, v_TextureCoord.st ).rgba * v_Color;" for GLES. I did this by storing these strings in a common variable texColorString which I added in OpenGL-Common/shader.h and I used this variable in the raw string which is returned in getDefaultFragmentShader function.

@RobertBColton tested these changes and below are the test results:


Shader fix commits: b8ec0bf

Task 1 completed.


TASK 2: ADDING A NEW AUDIO SYSTEM

We needed a new audio system that was able to work for android, so I created an SDL audio system with the help of SDL_mixer

I used SDL_mixer docs to get all the functions I need for this audio system like :

  • Mix_OpenAudio
  • Mix_PlayChannel
  • Mix_GetChunk
  • Mix_HaltChannel
  • Mix_Pause
  • Mix_Playing
  • Mix_Resume
  • Mix_Paused
  • Mix_SetPanning
  • Mix_QuerySpec
  • Mix_VolumeChunk
  • Mix_Volume
  • Mix_AllocateChannels

First I created SoundResource.h which has the sound structure and some functions like update, destroy etc. Then I created SDLsystem which has audiosystem_initialize() which is responsible for initializing the SDL mixer api using Mix_OpenAudio and loading support for different audio sample formats using Mix_Init.

After this I added sound functions in SDLbasic.cpp like sound_exists, sound_add, sound_play, sound_delete, sound_stop, sound_loop and few more. These functions are common for all the audio systems. I used AssetArray for sounds because it is easy to access sound objects using the id.

For example below is my sound_play function which uses an SDL_mixer function,

bool sound_play(int sound) {
  const Sound& snd = sounds.get(sound);
  if (Mix_PlayChannel(-1,snd.mc, 0) == -1) { return false; }
  return true;
}

In the function above, sounds is the AssetArray of Sound objects and I use the id passed as a parameter to get the Sound. The sound structure has a chunk pointer(mc) and Mix_PlayChannel is the mixer function for playing the chunk on a channel. The other basic functions are made using the same approach.


Once I finished the basic functions I started the audio functions. For audio functions, I added SoundChanel.h which had the SoundChannel structure which looked like

struct SoundChannel {
  Mix_Chunk *mchunk;
  int soundIndex;
  double priority;
};


SoundChannel is being used because most of the audio functions for the SDL audio system use channel id instead of sound id.
Just like sound functions the audio functions are common for all the audio systems. I created SDLadvanced.cpp
and added audio functions like audio_exists, audio_add, audio_play_sound, audio_stop_sound and few more.
The audio functions are made using the logic I used for sound functions. For example:

sound_ispaused:

bool sound_ispaused(int sound) {
  const Sound& snd = sounds.get(sound);
  int channel_count = Mix_AllocateChannels(-1);
  for(int i = 0;i <= channel_count ; i++) {
    if (Mix_GetChunk(i) == snd.mc && Mix_Paused(i) == 1) { return true; }
  }
  return false;
}


audio_is_paused:

bool audio_is_paused(int index) {
  if (index >= AUDIO_CHANNEL_OFFSET) {
    return Mix_Paused(index - AUDIO_CHANNEL_OFFSET);
  }
  const Sound& snd = sounds.get(index);
  for(size_t i = 0; i < sound_channels.size() ; i++) {
    if (sound_channels[i]->mchunk == snd.mc && Mix_Paused(i) == 1) { return true; }
  }
  return false;
}

logic is the same but one uses only sound while the other uses both sound and channel structures.

This is because sound basic is not intended to be multichannel, but advanced audio is intended to take full advantage of SDL_mixer. Audio functions are positional multichannel audio (much more powerful) & sound functions are kept for backwards compatibility. The audio_play_sound function returns the channel id, that way the user can store the id of the channel in a variable then all the volume/pan/stop/pause functions will take both sound id and sound channel id. Using the advanced audio functions, the user gets full control over each channel.

After finishing the required functions for both SDLbasic.cpp and SDLadvanced.cpp I started testing for both windows and android and I was able to hear the audio for both windows and android builds.

The tests were successful for FLAC, MP3, OGG, WAV audio file formats.


Audio system commits: c87994f, 0d55208

Task 2 completed.


TASK 3: ADDING THE TILT FEATURE

@fundies provided some useful links for this task like:
SDL_android.c & SDLActivity.java

after going through these files we decided to use Android_JNI_GetAccelerometerValues.

I started task 3 by first adding android.cpp in Platforms\SDL\Android and added 3 functions device_get_tilt_x, device_get_tilt_y & device_get_tilt_z and included SDL_android.h which is a requirement for Android_JNI_GetAccelerometerValues.
Then I added override CXXFLAGS += -I$(ENIGMA_ROOT)/android/external/SDL2/src/core/android/ & SOURCES += $(wildcard Platforms/SDL/Android/*.cpp) to the makefile present in Platforms\SDL. Below is the implementation of device_get_tilt_x

float device_get_tilt_x()
{ 
  float Values[3],tilt_x;
  Android_JNI_GetAccelerometerValues(Values);
  tilt_x = Values[0];
  return tilt_x;

}

The other 2 functions are similar to this and the only change is that tilt_y returns Values[1] and tilt_z returns Values[2].


I added the tilt feature in the first audio system commit: c87994f

Task 3 completed.


What's left / What will be done after GSoC?

  • In the Shader fix part the raw string needs to have texture2D for OpenGL1 and texture for GL3. It's the same for GLES2 and GLES3.
  • Addition of extra audio functions in the SDL audio system for effects etc.
  • Tilt has some delay which needs to be fixed.
  • I will try to implement the suggestions given by @RobertBColton in the comments below.

fundies and others added 11 commits July 6, 2021 16:52
* SDL scaling fix
* removed extra tabs,spaces and newlines
* fix for clearing garbage
* Adding SDL audio_system

* added length and volume functions for SDL audio_system

* fixing volume functions for SDL audio_system

* Adding audio functions in the SDL audio_system

* Improving advanced and basic sound functions for SDL audio_system

* Adding tilt
* fixing audio functions and cleaning up code for SDL audio_system

* fixed about.ey in SDL audio_system

* Update ENIGMAsystem/SHELL/Audio_Systems/SDL/SDLadvanced.cpp

Co-authored-by: Robert Colton <robertbcolton@gmail.com>

* Update SDLbasic.cpp

* Update SDLsystem.cpp

Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Copy link
Contributor

@RobertBColton RobertBColton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a few things that should be cleaned up yet, some of it before this is merged.

ENIGMAsystem/SHELL/Audio_Systems/SDL/Makefile Outdated Show resolved Hide resolved
ENIGMAsystem/SHELL/Audio_Systems/SDL/SDLadvanced.cpp Outdated Show resolved Hide resolved
ENIGMAsystem/SHELL/Audio_Systems/SDL/SDLbasic.cpp Outdated Show resolved Hide resolved
ENIGMAsystem/SHELL/Audio_Systems/SDL/SDLadvanced.cpp Outdated Show resolved Hide resolved
ENIGMAsystem/SHELL/Audio_Systems/SDL/SDLadvanced.cpp Outdated Show resolved Hide resolved

override LDLIBS += -lepoxy
override CXXFLAGS += -IBridges/OpenGLES -IGraphics_Systems/OpenGLES/wrappers
override CFLAGS += -I$(ENIGMA_ROOT)/android/external/glm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be moved into a TARGET-PLATFORM Android check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobertBColton I think I will look into this after GSoC

ENIGMAsystem/SHELL/Graphics_Systems/OpenGLES3/Makefile Outdated Show resolved Hide resolved
ENIGMAsystem/SHELL/Graphics_Systems/OpenGLES2/Makefile Outdated Show resolved Hide resolved
Comment on lines +1 to +75
#include "Graphics_Systems/OpenGL-Common/surface_impl.h"
#include "Graphics_Systems/OpenGL-Common/textures_impl.h"
#include "OpenGLHeaders.h"
#include "Graphics_Systems/General/GSsurface.h"
#include "Graphics_Systems/General/GSprimitives.h"
#include "Graphics_Systems/General/GSscreen.h"
#include "Graphics_Systems/General/GSmatrix.h"
#include "Graphics_Systems/General/GStextures.h"
#include "Graphics_Systems/General/GStextures_impl.h"
#include "Graphics_Systems/graphics_mandatory.h"

#ifdef DEBUG_MODE
#include "Widget_Systems/widgets_mandatory.h" // for show_error
#endif

namespace enigma_user{

void surface_add_colorbuffer(int id, int index, int internalFormat, unsigned format, unsigned type){
get_surface(surf,id);
glBindFramebuffer(GL_FRAMEBUFFER, surf.fbo);
int texture = enigma::graphics_create_texture_custom(enigma::RawImage(nullptr, surf.width, surf.height), false, nullptr, nullptr, internalFormat, format, type);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0+index, GL_TEXTURE_2D, enigma::get_texture_peer(texture), 0);
glBindFramebuffer(GL_FRAMEBUFFER, enigma::bound_framebuffer);
}

void surface_add_depthbuffer(int id, int internalFormat, unsigned format, unsigned type){
get_surface(surf,id);
glBindFramebuffer(GL_FRAMEBUFFER, surf.fbo);
int texture = enigma::graphics_create_texture_custom(enigma::RawImage(nullptr, surf.width, surf.height), false, nullptr, nullptr, internalFormat, format, type);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, enigma::get_texture_peer(texture), 0);
glBindFramebuffer(GL_FRAMEBUFFER, enigma::bound_framebuffer);
}

void surface_set_target(int id)
{
draw_batch_flush(batch_flush_deferred);

get_surface(surf,id);
//This fixes several consecutive surface_set_target() calls without surface_reset_target.
if (enigma::bound_framebuffer != 0) { d3d_transform_stack_pop(); d3d_projection_stack_pop();}
enigma::bound_framebuffer = surf.fbo;
glBindFramebuffer(GL_FRAMEBUFFER, surf.fbo); //bind it
d3d_transform_stack_push();
d3d_projection_stack_push();
glViewport(0, 0, surf.width, surf.height);
glScissor(0, 0, surf.width, surf.height);
d3d_set_projection_ortho(0, surf.height, surf.width, -surf.height, 0);
}

void surface_reset_target(void)
{
draw_batch_flush(batch_flush_deferred);
enigma::bound_framebuffer = 0;
glBindFramebuffer(GL_FRAMEBUFFER, 0);
d3d_transform_stack_pop();
d3d_projection_stack_pop();
screen_reset_viewport();
}

void surface_free(int id)
{
get_surface(surf,id);
if (enigma::bound_framebuffer == surf.fbo) glBindFramebuffer(GL_FRAMEBUFFER, enigma::bound_framebuffer=0);
enigma::graphics_delete_texture(surf.texture);
if (surf.write_only == true){
if (surf.has_depth_buffer == true) { glDeleteRenderbuffers(1, &surf.depth_buffer); }
if (surf.has_stencil_buffer == true) { glDeleteRenderbuffers(1, &surf.stencil_buffer); }
}else if (surf.has_depth_buffer == true){
enigma::graphics_delete_texture(surf.depth_buffer);
}
glDeleteFramebuffers(1, &surf.fbo);
delete enigma::surfaces[id];
}

}// namespace enigma_user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we actually don't need to duplicate these. It would be ok to put the GLES version back in common, and it would work fine on newer OpenGL. While using separate draw/read buffers is a newer feature, our code actually wasn't making a real use of it before, so using the combined GL_FRAMEBUFFER target that works in old and new GL would be fine in common.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobertBColton I think I will look into this after GSoC

Comment on lines 1 to 50
#include "shader_wrappers.h"

void glUniform1ui(int location, unsigned v0)
{
glUniform1i(location ,v0);

}

void glUniform2ui(int location, unsigned v0, unsigned v1)
{
glUniform2i(location , v0,v1);

}

void glUniform3ui(int location, unsigned v0, unsigned v1, unsigned v2)
{
glUniform3i(location,v0,v1,v2);

}

void glUniform4ui(int location, unsigned v0, unsigned v1, unsigned v2, unsigned v3)
{
glUniform4i(location, v0, v1, v2, v3);

}

//for unsigned int uniforms

void glUniform1uiv(int location, int size, const GLuint* value)
{
glUniform1iv(location,size, (const GLint*)value);

}

void glUniform2uiv(int location, int size, const GLuint* value)
{
glUniform2iv(location,size,(const GLint*)value);

}

void glUniform3uiv(int location, int size,const GLuint* value)
{
glUniform3iv(location,size,(const GLint*)value);

}

void glUniform4uiv(int location, int size, const GLuint* value)
{
glUniform4iv(location,size,(const GLint*)value);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're actually going to need a better solution to this one. Passing unsigned as signed ints is just going to overflow. GameMaker Studio 2, being dynamically typed, only exposes shader_set_uniform_i and doesn't actually expose the unsigned version of these functions. What we actually need to do here is split it so that only GL3/GLES3 and newer use the unsigned functions. Then we need to completely remove them from GLES2/GL1.

Look at how these functions are actually being used by the engine to see how to remove them from GL1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. If you're really worried about overflows just put a std::min(v, int_max) or whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobertBColton @fundies I think I will look into this after GSoC

diabl0-NEMESIS and others added 13 commits August 18, 2021 20:17
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
Co-authored-by: Robert Colton <robertbcolton@gmail.com>
float device_get_tilt_z()
{
float Values[3],tilt_z;
Android_JNI_GetAccelerometerValues(Values);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the tilt is actually too slow, then it may need to be cached like we do with window caption. Alternatively, we could just poll it once every tick from a callback in the main loop. You'd store that array in namespace enigma and have these functions return enigma::tilt_values[...] instead.

@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #2261 (0af3d9a) into master (953d4c6) will increase coverage by 0.00%.
The diff coverage is 69.23%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2261   +/-   ##
=======================================
  Coverage   35.22%   35.23%           
=======================================
  Files         211      214    +3     
  Lines       20413    20450   +37     
=======================================
+ Hits         7191     7205   +14     
- Misses      13222    13245   +23     
Impacted Files Coverage Δ
...NIGMAsystem/SHELL/Audio_Systems/OpenAL/ALbasic.cpp 0.64% <ø> (+<0.01%) ⬆️
...Asystem/SHELL/Graphics_Systems/General/GSmodel.cpp 48.35% <ø> (ø)
...m/SHELL/Graphics_Systems/OpenGL-Common/scissor.cpp 0.00% <0.00%> (ø)
...em/SHELL/Graphics_Systems/OpenGL-Common/screen.cpp 53.33% <ø> (-19.75%) ⬇️
...m/SHELL/Graphics_Systems/OpenGL-Common/surface.cpp 0.00% <ø> (-34.62%) ⬇️
.../SHELL/Graphics_Systems/OpenGL-Common/textures.cpp 97.61% <ø> (-0.39%) ⬇️
...ELL/Graphics_Systems/OpenGL-Common/textures_impl.h 100.00% <ø> (ø)
ENIGMAsystem/SHELL/Platforms/General/PFmain.cpp 81.25% <ø> (ø)
ENIGMAsystem/SHELL/Platforms/SDL/Event.cpp 17.93% <0.00%> (-0.82%) ⬇️
ENIGMAsystem/SHELL/SHELLmain.cpp 100.00% <ø> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 953d4c6...0af3d9a. Read the comment docs.

@fundies fundies dismissed RobertBColton’s stale review August 30, 2021 02:22

cause he's a goober

@fundies fundies merged commit e65b969 into enigma-dev:master Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants