Skip to content

Conversation

codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 29, 2025

PR Summary

Code Refactoring and Bug Fixes Across Multiple Components

Overview

This PR refactors several components to improve code quality and fix various issues. It includes optimization of the download progress reporting system, fixes for UI components, removal of deprecated code, and native JNI modifications.

Change Types

Type Description
Refactor Improved code structure in CustomSeekbar and MinecraftDownloader
Bugfix Fixed thumb positioning in CustomSeekbar and corrected typos
Enhancement Added download speed calculation and Android Oreo compatibility
Removal Removed unused code and deprecated Android 10 workarounds

Affected Modules

Module / File Change Description
src/main/java/com/kdt/CustomSeekbar.java Refactored setup() method and fixed thumb positioning
src/main/java/net/kdt/pojavlaunch/Tools.java Removed unused import statement
src/main/java/net/kdt/pojavlaunch/mirrors/DownloadMirror.java Fixed typo in log message
src/main/java/net/kdt/pojavlaunch/prefs/QuickSettingSideDialog.java Removed resolution bar scale limits
src/main/java/net/kdt/pojavlaunch/tasks/MinecraftDownloader.java Refactored variables and improved download progress reporting with speed calculation
src/main/jni/ctxbridges/gl_bridge.c Added new JNI function for GL4ES internals initialization
src/main/jni/input_bridge_v3.c Removed deprecated functions and changed linker hook implementation

Notes for Reviewers

  • The resolution bar's minimum/maximum scale limits removal might allow unsafe scaling levels
  • Verify the Android 10 linker bug mitigation removal doesn't cause compatibility issues
  • Check that the new GL4ES initialization works properly across different devices

Additional Context

  • The changes to MinecraftDownloader improve download metrics visibility
  • CustomSeekbar changes add compatibility for Android 8.0 (Oreo) devices

Mathias-Boulay and others added 15 commits December 27, 2024 12:31
- Add download size queries through a HEAD request
- Use the file size for progress instead of file count when all file size are available
- Add download speed meter
Update GPLAY_PRIVACY_POLICY to be more clear, and remove the news entry as the news in Pojav no longer exists.
Turns out if you were perfectly still, it would fail. For a quick tap, it was fairly easy to create.
NOTE: requires a different version of the FFmpeg plugin which I haven't made yet
void (*set_getmainfbsize)(void (*new_getMainFBSize)(int* width, int* height)) = (void*)GETSYM("set_getmainfbsize");
if(set_getmainfbsize != NULL) {
__android_log_print(ANDROID_LOG_INFO, g_LogTag, "GL4ES internals initialized dimension callback");
set_getmainfbsize(gl4esi_get_display_dimensions);
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Undefined function reference.

The code calls gl4esi_get_display_dimensions which appears to be undefined or not declared in the visible code, potentially causing compilation errors or runtime crashes.

Current Code (Diff):

- |8 spaces|         set_getmainfbsize(gl4esi_get_display_dimensions);
+ |8 spaces|         // Ensure gl4esi_get_display_dimensions is properly defined before using it
+ |8 spaces|         set_getmainfbsize(gl4esi_get_display_dimensions);
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
set_getmainfbsize(gl4esi_get_display_dimensions);
// Ensure gl4esi_get_display_dimensions is properly defined before using it
set_getmainfbsize(gl4esi_get_display_dimensions);

__android_log_print(ANDROID_LOG_INFO, g_LogTag, "GL4ES internals initializing...");
jclass funcProviderClass = (*env)->GetObjectClass(env, function_provider);
jmethodID method_getFunctionAddress = (*env)->GetMethodID(env, funcProviderClass, "getFunctionAddress", "(Ljava/lang/CharSequence;)J");
#define GETSYM(N) ((*env)->CallLongMethod(env, function_provider, method_getFunctionAddress, (*env)->NewStringUTF(env, N)));
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

JNI local reference leak.

The NewStringUTF call creates a local reference that's not released, which could lead to local reference table overflow in loops or long-running operations.

Current Code (Diff):

- #define GETSYM(N) ((*env)->CallLongMethod(env, function_provider, method_getFunctionAddress, (*env)->NewStringUTF(env, N)));
+ #define GETSYM(N) ({ jstring str = (*env)->NewStringUTF(env, N); long result = (*env)->CallLongMethod(env, function_provider, method_getFunctionAddress, str); (*env)->DeleteLocalRef(env, str); result; })
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
#define GETSYM(N) ((*env)->CallLongMethod(env, function_provider, method_getFunctionAddress, (*env)->NewStringUTF(env, N)));
#define GETSYM(N) ({ jstring str = (*env)->NewStringUTF(env, N); long result = (*env)->CallLongMethod(env, function_provider, method_getFunctionAddress, str); (*env)->DeleteLocalRef(env, str); result; })

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants