search

MatteoBattilana / WeatherView

WeatherView is an Android Library let you make cool weather animations for your app
Apache License 2.0
506 stars 81 forks source link

Library overhaul #9

Closed magneticflux- closed 7 years ago

magneticflux- commented 7 years ago

@MatteoBattilana I'd like to know your thoughts on what I have so far. I know I still have several points of functionality to re-implement, but is there anything that is concerning to you immediately or do you see any opportunities to easily add features?

Side note: Android Studio 3.0 Canary 5 is probably required to build properly.

Closes #7, closes #8

Things to do before merge:

MatteoBattilana commented 7 years ago

Hi, really thank to you and what you're doing for this library. I'm working hard on a university project, I'll look and test the code asps. I'm also waiting for new commits.

Thanks

magneticflux- commented 7 years ago

@MatteoBattilana I think that the branch is ready for your review. I'd request one using GitHub, but I don't have the required permissions so you'll just have to start a review on your own.

MatteoBattilana commented 7 years ago

@magneticflux- I'm testing the library, I think this is a great work. In my opinion we have to reduce the speed of both animations and the starting value for the fade out percentage, I think 370 is a good value for the rain speed and 220 for the snow. I'm also thinking 0.75f is the right value for the fade out percentage. What do you think?

Another thing; when the orientation is enable, everything is working correctly but in current master branch I've put a limit to the angle. For example, this is necessary when the phone is on the table.

magneticflux- commented 7 years ago

@MatteoBattilana I'm thinking that 220 for the snow speed is fine, but it looks a little unnatural for rain to be falling so slowly. How about 800 for rain instead of 2000? I'm also working on a more efficient drawing method as well as motion blur, so that might make higher speeds look better.

magneticflux- commented 7 years ago

After doing some research I have found relative values for rain and snow speeds:

Taking those averages, we have 5.5 m/s for rain and 1.5 m/s for snow. Therefore, the rain speed should be about 3.66x faster than the snow speed. To my surprise, that means that the correct "to-scale" rain velocity would be just over 800!

magneticflux- commented 7 years ago

Here are two comparisons of the graphics. The old bitmaps are on the left, and the new drawing method is on the right.

2017-07-11 2017-07-11

MatteoBattilana commented 7 years ago

You made a wonderful job, testing the demo application I realized that the snow and rain speeds are well set.

I run in a bug when I tested the demo in an older device running Android 4.3. I got:

 com.github.matteobattilana.weather E/AndroidRuntime: FATAL EXCEPTION: main
  Process: com.github.matteobattilana.weather, PID: 23337
  java.lang.RuntimeException: Unable to start activity ComponentInfo{com.github.matteobattilana.weather/com.github.matteobattilana.demo.MainActivity}: android.content.res.Resources$NotFoundException: Resource ID #0x7f070052
      at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2328)
      at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2386)
      at android.app.ActivityThread.access$900(ActivityThread.java:169)
      at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1277)
      at android.os.Handler.dispatchMessage(Handler.java:102)
      at android.os.Looper.loop(Looper.java:136)
      at android.app.ActivityThread.main(ActivityThread.java:5476)
      at java.lang.reflect.Method.invokeNative(Native Method)
      at java.lang.reflect.Method.invoke(Method.java:515)
      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1283)
      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099)
      at dalvik.system.NativeStart.main(Native Method)
   Caused by: android.content.res.Resources$NotFoundException: Resource ID #0x7f070052
      at android.content.res.Resources.getValue(Resources.java:2057)
      at android.support.v7.widget.AppCompatDrawableManager.loadDrawableFromDelegates(AppCompatDrawableManager.java:330)
      at android.support.v7.widget.AppCompatDrawableManager.getDrawable(AppCompatDrawableManager.java:195)
      at android.support.v7.widget.AppCompatDrawableManager.getDrawable(AppCompatDrawableManager.java:188)
      at android.support.v7.widget.AppCompatDrawableManager.checkVectorDrawableSetup(AppCompatDrawableManager.java:755)
      at android.support.v7.widget.AppCompatDrawableManager.getDrawable(AppCompatDrawableManager.java:193)
      at android.support.v7.widget.TintTypedArray.getDrawableIfKnown(TintTypedArray.java:88)
      at android.support.v7.app.AppCompatDelegateImplBase.<init>(AppCompatDelegateImplBase.java:128)
      at android.support.v7.app.AppCompatDelegateImplV9.<init>(AppCompatDelegateImplV9.java:149)
      at android.support.v7.app.AppCompatDelegateImplV11.<init>(AppCompatDelegateImplV11.java:29)
      at android.support.v7.app.AppCompatDelegateImplV14.<init>(AppCompatDelegateImplV14.java:54)
      at android.support.v7.app.AppCompatDelegate.create(AppCompatDelegate.java:202)
      at android.support.v7.app.AppCompatDelegate.create(AppCompatDelegate.java:183)
      at android.support.v7.app.AppCompatActivity.getDelegate(AppCompatActivity.java:520)
      at android.support.v7.app.AppCompatActivity.onCreate(AppCompatActivity.java:71)
      at com.github.matteobattilana.demo.MainActivity.onCreate(MainActivity.kt:24)
      at android.app.Activity.performCreate(Activity.java:5451)
      at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1093)
      at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2292)
      at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2386) 
      at android.app.ActivityThread.access$900(ActivityThread.java:169) 
      at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1277) 
      at android.os.Handler.dispatchMessage(Handler.java:102) 
      at android.os.Looper.loop(Looper.java:136) 
      at android.app.ActivityThread.main(ActivityThread.java:5476) 
      at java.lang.reflect.Method.invokeNative(Native Method) 
      at java.lang.reflect.Method.invoke(Method.java:515) 
      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1283) 
      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099) 
      at dalvik.system.NativeStart.main(Native Method)

Everything works correctly with Android 7.0 or higher

I checked the code and I think the problem is:

lateinit var weatherSensor: WeatherViewSensorEventListener

Once this bug is solved, I'll merge this branch into the master.

magneticflux- commented 7 years ago

@MatteoBattilana It actually looks like a bizarre problem with the support libraries... It's crashing in onCreate because it can't find a drawable on earlier version of Android. I'm looking into the issue to see if we can implement a workaround or just use an earlier support library version.

I removed the orientation sensor to make sure that wasn't the problem and the error persisted. I'm worried that it's a Support Library bug, but I can't find anyone else experiencing it...

This is the relevant location where it first tries to load the missing drawable:

at android.support.v7.widget.AppCompatDrawableManager.checkVectorDrawableSetup(AppCompatDrawableManager.java:755)
MatteoBattilana commented 7 years ago

I saw you solved the issue. I've tested the library and found the same bug on 4.4 version. I'll merge now your pull request, we'll fix this issue asap.