ditek / MidiSheetMusic-Android

Play and visualize MIDI music files
GNU General Public License v3.0
130 stars 51 forks source link

Feature full height and red dash #25

Closed caveman77 closed 2 years ago

caveman77 commented 2 years ago

Good morning, I hope that you are doing well taking account the pendemic situation.

I would like to propose here some new content for your Extended Midi Sheet Music app: 1/ Dash as red: This feature enable the use in one click to put all flat & dash in red 2/ Delay before start: This feature enable the user to have the music start with a delay 3/ Full height: This feature enarge the score to the full height of the screen

All the 3 features are not activated by default and are requiring the user to go to setting (either 1st panel or second). I tried to reduce as much as possible the changes. I didn't added any dependency. On the delay feature, I initially relied on options.timeshift but it didn't worked as expected. Then I made it simple and added a delay on the play button.

Any question, just let me know.

Happy new year ! Christian.

ditek commented 2 years ago

Thanks! I'll take a look and let you know :)

caveman77 commented 2 years ago

You're welcome. I m using it on daily basis. I may have forgotten to put the delay as persistent. Feel free to modify it. Wording might not be the best (in the settings I added). Here as well, feel free to modify.

I have some other ideas like bookmarks. I might come later on another pull request. Any question, just let me know.

caveman77 commented 2 years ago

About the main comment, It was intentional. I was willing to have the full heigh whenever the play button is pressed and not before. Now, I agree it is bringing confusion. I ll simply keep the play bar space into the calculation. As this is mainly the keyboard taking space, it will be fine enough.

caveman77 commented 2 years ago

You're correct. "Dash" word is not something that I have should have used but "sharp". The feature is helping but telling the player to press a black key on the keyboard. As the signature can be gone from a while, I find it usefull to have this as a reminder. Some web site are proposing this as "enhance" feature. https://en.wikipedia.org/wiki/Accidental_(music)

caveman77 commented 2 years ago

I have an issue as I'm not able to test it with my phone with the initial layout.

image

caveman77 commented 2 years ago

I found a new layout configuration that is working for all phones as far as I can see. I ll push it in the next pull request

caveman77 commented 2 years ago

"The sheet would get blown out if you go to "More settings" and navigate back even if you don't enable "Use full height". This would happen more often if you enable note colors or dash colors. This was only experienced on device 1. Device 2 didn't have this issue." I succeed to reproduce it wit the emulator. I'll take a look.

caveman77 commented 2 years ago

I took a look. the function getPreferredSize in MidiPlayer.java is receiving switched inputs. Height instead of width. The player height calculation is therefore wrong and further zoom calculation as well. As this activity is horizontal and the more setting menu is vertical, I guess that sometimes Android is in the middle of switching the screen position. I ll take a look if I find any fix for this. Please note that this issue is not related to the pull request.

caveman77 commented 2 years ago

You have now a new commit on this pull request. Kind regards.

ditek commented 2 years ago

Do you have certain config to replicate your Galaxy S7 using the emulator? I can add it to the devices I test with in the future. When it comes to accidentals, it would be nice (in a future PR) to make the setting control all of them, not just the sharps.

ditek commented 2 years ago

I took a look. the function getPreferredSize in MidiPlayer.java is receiving switched inputs. Height instead of width. The player height calculation is therefore wrong and further zoom calculation as well. As this activity is horizontal and the more setting menu is vertical, I guess that sometimes Android is in the middle of switching the screen position.

You're probably right. I saw that you added an 'if' statement to handle that. It doesn't happen anymore. Nice!

ditek commented 2 years ago

I fixed a bug with calculating the zoom (dividing two integers and then casting to a float) and added handling the player bar. I'll try to commit them to the PR soon.

caveman77 commented 2 years ago

Do you have certain config to replicate your Galaxy S7 using the emulator? I can add it to the devices I test with in the future. When it comes to accidentals, it would be nice (in a future PR) to make the setting control all of them, not just the sharps.

I do think that I reproduced the issue with the emulator on my PC. I followed this link to created it: https://developer.samsung.com/galaxy-emulator-skin/guide.html

Samsung Galaxy S7 API 26 5.1' 1440x2560px Android 8.0 x86

I'm pretty sure the issue is related to the size of the phone.

Regarding the sharps, the development is changing already all the accidentals. Not only the sharps. Either sharp or fats next to signature or not. Anytime that you need to press a black key on the keyboard

ditek commented 2 years ago

Regarding the sharps, the development is changing already all the accidentals. Not only the sharps. Either sharp or fats next to signature or not. Anytime that you need to press a black key on the keyboard

I see. Then I'll rename things to reflect that.

ditek commented 2 years ago

I pushed the changes. I also find a way to get the speed bar to expand while keeping other buttons visible. Seems to work on the S7 config you sent me, but let me know if it works with your device.

ditek commented 2 years ago

In a future version I'll probably make the "full height" mode the default. I might also move the "Accidental Colors" option inside "More settings" as I don't imagine it needs to be changed often.

caveman77 commented 2 years ago

With the last commit, on my S7 the setting button is very cut almost in the middle. That's fine for me but some user might have difficulties Screenshot_20220204-213227_Enhanced MidiSheetMusic

ditek commented 2 years ago

It didn't look like that when I tested. Let me check it again.

ditek commented 2 years ago

Looks like this on the S7 emulator, so it's probably different on a real device: image

ditek commented 2 years ago

Could you try removing line 102 (android:layout_weight="0.5") in player_toolbar.xml and let me know how it looks?

caveman77 commented 2 years ago

Could you try removing line 102 (android:layout_weight="0.5") in player_toolbar.xml and let me know how it looks?

I'm afraid this is worse. WhatsApp Image 2022-02-06 at 12 01 53

ditek commented 2 years ago

I'm still unable to reproduce this in my emulator. It would be great if you could find an emulator setting that would reproduce that. I'm sure I'll be able to fix it then.

ditek commented 2 years ago

How does it look like if we set line 102 back to how you did it: android:layout_width="wrap_content"

caveman77 commented 2 years ago

I'm still unable to reproduce this in my emulator. It would be great if you could find an emulator setting that would reproduce that. I'm sure I'll be able to fix it then.

I can reproduice it with the simulator. If I m doing an export of the device it will take few Mb. Is there a way to export it in light way ?

caveman77 commented 2 years ago

android:layout_width="wrap_content"

With this:

    <ImageButton
        android:id="@+id/btn_settings"
        style="@style/ToolbarButtonStyle"
        android:layout_weight="0.5"
        android:layout_width="wrap_content"
        android:contentDescription="@string/settings"
        android:src="@drawable/ic_settings" />
</LinearLayout>

The outcome is perfect.

ditek commented 2 years ago

I can reproduice it with the simulator. If I m doing an export of the device it will take few Mb. Is there a way to export it in light way ?

I don't really know. Here is the config for my simulator:

Name: Galaxy_S7_API_26
CPU/ABI: x86
Path: .../.android/avd/Galaxy_S7_API_26.avd
Target: google_apis [Google APIs] (API level 26)
Skin: 2560x1440
SD Card: 512M
AvdId: Galaxy_S7_API_26
PlayStore.enabled: false
avd.ini.displayname: Galaxy S7 API 26
avd.ini.encoding: UTF-8
disk.dataPartition.size: 800M
fastboot.chosenSnapshotFile: 
fastboot.forceChosenSnapshotBoot: no
fastboot.forceColdBoot: no
fastboot.forceFastBoot: yes
hw.accelerometer: yes
hw.arc: false
hw.audioInput: yes
hw.battery: yes
hw.camera.back: virtualscene
hw.camera.front: emulated
hw.cpu.ncore: 4
hw.dPad: no
hw.device.hash2: MD5:ce3479a56e11e779f9fe763f32ee8bc7
hw.device.manufacturer: User
hw.device.name: Galaxy S7
hw.gps: yes
hw.gpu.enabled: yes
hw.gpu.mode: auto
hw.initialOrientation: landscape
hw.keyboard: yes
hw.lcd.density: 640
hw.lcd.height: 2560
hw.lcd.width: 1440
hw.mainKeys: no
hw.ramSize: 1536
hw.sdCard: yes
hw.sensors.orientation: yes
hw.sensors.proximity: yes
hw.trackBall: no
image.sysdir.1: system-images/android-26/google_apis/x86/
runtime.network.latency: none
runtime.network.speed: full
showDeviceFrame: no
skin.dynamic: yes
skin.path.backup: _no_skin
tag.display: Google APIs
tag.id: google_apis
vm.heapSize: 512
ditek commented 2 years ago

I committed the change.

caveman77 commented 2 years ago

I can reproduice it with the simulator. If I m doing an export of the device it will take few Mb. Is there a way to export it in light way ?

I don't really know. Here is the config for my simulator:

Name: Galaxy_S7_API_26
CPU/ABI: x86
Path: .../.android/avd/Galaxy_S7_API_26.avd
Target: google_apis [Google APIs] (API level 26)
Skin: 2560x1440
SD Card: 512M
AvdId: Galaxy_S7_API_26
PlayStore.enabled: false
avd.ini.displayname: Galaxy S7 API 26
avd.ini.encoding: UTF-8
disk.dataPartition.size: 800M
fastboot.chosenSnapshotFile: 
fastboot.forceChosenSnapshotBoot: no
fastboot.forceColdBoot: no
fastboot.forceFastBoot: yes
hw.accelerometer: yes
hw.arc: false
hw.audioInput: yes
hw.battery: yes
hw.camera.back: virtualscene
hw.camera.front: emulated
hw.cpu.ncore: 4
hw.dPad: no
hw.device.hash2: MD5:ce3479a56e11e779f9fe763f32ee8bc7
hw.device.manufacturer: User
hw.device.name: Galaxy S7
hw.gps: yes
hw.gpu.enabled: yes
hw.gpu.mode: auto
hw.initialOrientation: landscape
hw.keyboard: yes
hw.lcd.density: 640
hw.lcd.height: 2560
hw.lcd.width: 1440
hw.mainKeys: no
hw.ramSize: 1536
hw.sdCard: yes
hw.sensors.orientation: yes
hw.sensors.proximity: yes
hw.trackBall: no
image.sysdir.1: system-images/android-26/google_apis/x86/
runtime.network.latency: none
runtime.network.speed: full
showDeviceFrame: no
skin.dynamic: yes
skin.path.backup: _no_skin
tag.display: Google APIs
tag.id: google_apis
vm.heapSize: 512

Below mine. I don't know if it is having any impact but height and width are mixed up.

Name: Samsung_Galaxy_S7_API_26 CPU/ABI: Google APIs Intel Atom (x86) Path: C:\Users\Christian.android\avd\Samsung_Galaxy_S7_API_26.avd Target: google_apis [Google APIs] (API level 26) Skin: 1440x2560 fastboot.chosenSnapshotFile: runtime.network.speed: full hw.accelerometer: yes hw.device.name: Samsung Galaxy S7 hw.lcd.width: 1440 hw.initialOrientation: Portrait image.androidVersion.api: 26 tag.id: google_apis hw.mainKeys: yes hw.camera.front: emulated avd.ini.displayname: Samsung Galaxy S7 API 26 hw.gpu.mode: auto hw.ramSize: 1536 PlayStore.enabled: false fastboot.forceColdBoot: no hw.cpu.ncore: 2 hw.keyboard: yes hw.sensors.proximity: yes hw.dPad: no hw.lcd.height: 2560 vm.heapSize: 512 skin.dynamic: yes hw.device.manufacturer: User hw.gps: yes skin.path.backup: _no_skin hw.audioInput: yes image.sysdir.1: system-images\android-26\google_apis\x86\ showDeviceFrame: no hw.camera.back: virtualscene AvdId: Samsung_Galaxy_S7_API_26 hw.lcd.density: 640 hw.arc: false hw.device.hash2: MD5:39ffb355d6a04ad51b5c494245f4dc82 fastboot.forceChosenSnapshotBoot: no fastboot.forceFastBoot: yes hw.trackBall: no hw.battery: yes hw.sdCard: yes tag.display: Google APIs runtime.network.latency: none disk.dataPartition.size: 800M hw.sensors.orientation: yes avd.ini.encoding: UTF-8 hw.gpu.enabled: yes

ditek commented 2 years ago

I don't know if it is having any impact but height and width are mixed up.

I believe that's just because I have set my default orientation to landscape while yours is portrait.

ditek commented 2 years ago

Does the latest commit work for you?

ditek commented 2 years ago

The only differences seem with the number of cores and that you're running Windows while I run MacOS. Maybe the OS makes a difference somehow.

caveman77 commented 2 years ago

Does the latest commit work for you?

Setting button is perfect on this commit for my S7.

ditek commented 2 years ago

Fantastic! Merging the PR :)

caveman77 commented 2 years ago

Fantastic! Merging the PR :)

Wounderful ! Thanks