bitcoin-core / gui-qml

Bitcoin GUI (experimental QML-based fork)
MIT License
107 stars 40 forks source link

android: patch getDrawable in QtActivityDelegate.java #312

Closed johnny9 closed 1 year ago

johnny9 commented 1 year ago

The getDrawable method used in the QtActivityDelegate has been deprecated and causes a runtime exception. The patch updates the method to include the theme as a second parameter so it can fully resolve the resources.

Windows Intel macOS Apple Silicon macOS ARM64 Android

johnny9 commented 1 year ago

This should resolve the runtime exception when calling createSurface

johnny9 commented 1 year ago

Update from 34d5b57 to 5116ee:

johnny9 commented 1 year ago

@hebasto have to apply a patch in a different part of the build with this one. Want to check if you have any thoughts on it.

hebasto commented 1 year ago

Mind providing more context please?

The getDrawable method used in the QtActivityDelegate has been deprecated

When has it been deprecated? In which Qt version?

... and causes a runtime exception.

Why does our implementation rely on a deprecated method?

The patch updates the method to include the theme as a second parameter so it can fully resolve the resources.

Is this patch available upstream?

johnny9 commented 1 year ago

Mind providing more context please?

The getDrawable method used in the QtActivityDelegate has been deprecated

When has it been deprecated? In which Qt version?

... and causes a runtime exception.

Why does our implementation rely on a deprecated method?

The patch updates the method to include the theme as a second parameter so it can fully resolve the resources.

Is this patch available upstream?

The function is deprecated in Android. https://developer.android.com/reference/android/content/res/Resources#getDrawable(int). The deprecated function is used in QtActivityDelegate.java which is a class provided by the Qt 5.15 source code. We pull it directly from the tarball and our BitcoinQtActivity derives from this class. The specific method causes a crash on the versions of android we have tested(8 and 13).

It was fixed in Qt 6.4 with this patch https://github.com/qt/qtbase/commit/e80b0f3d87a6f702b9d1d58068f8cc714c163c01 this patch doesn't directly apply to Qt 5.15 so I've created the patch myself.

johnny9 commented 1 year ago

Update from 5116ee9 to 459ebac

hebasto commented 1 year ago

Mind providing more context please?

The getDrawable method used in the QtActivityDelegate has been deprecated

When has it been deprecated? In which Qt version?

... and causes a runtime exception.

Why does our implementation rely on a deprecated method?

The patch updates the method to include the theme as a second parameter so it can fully resolve the resources.

Is this patch available upstream?

The function is deprecated in Android. https://developer.android.com/reference/android/content/res/Resources#getDrawable(int). The deprecated function is used in QtActivityDelegate.java which is a class provided by the Qt 5.15 source code. We pull it directly from the tarball and our BitcoinQtActivity derives from this class. The specific method causes a crash on the versions of android we have tested(8 and 13).

It was fixed in Qt 6.4 with this patch qt/qtbase@e80b0f3 this patch doesn't directly apply to Qt 5.15 so I've created the patch myself.

Thanks!

Mind putting these useful details into the PR description?

From https://bugreports.qt.io/browse/QTBUG-103568, it follows that a patch for Qt 5.15 does exist. But only for 5.15.11 which is source-closed for now.

Going to review your patch shortly.

johnny9 commented 1 year ago

Update from 459ebac to f1fedfe

hebasto commented 1 year ago

@jarolrod Can you confirm that this PR fixes the issue?

ping :)