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] compute HasRtlSupport once on startup #3801

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

jonathanpeppers
Copy link
Member

Description of Change

Previously we were calculating:

public static bool HasRtlSupport(this Context self)
{
    if (self == null)
        return false;

    return (self.ApplicationInfo?.Flags & AApplicationInfoFlags.SupportsRtl) == AApplicationInfoFlags.SupportsRtl;
}

Multiple times for every Label, Entry, or related controls.
Context.ApplicationInfo pulls values from the AndroidManifest.xml
for the entire Android application, so this isn't a value that would
not change after startup -- or for different controls.

https://developer.android.com/reference/android/content/pm/ApplicationInfo#flags
https://developer.android.com/reference/android/content/pm/ApplicationInfo#FLAG_SUPPORTS_RTL

Let's move this method to be a static readonly field in a new static
class. It will lazily be evaluated when the new class is accessed, and
only once per application run.

Results

Running ViewHandlerBenchmark on a Pixel 5 device:

> .\bin\dotnet\dotnet build .\src\Core\tests\Benchmarks.Droid\Benchmarks.Droid.csproj -t:Benchmark -c Release
Method Mean Error StdDev Gen 0 Allocated
-Label 527.6 µs 4.89 µs 4.57 µs 0.9766 5 KB
+Label 494.9 µs 5.72 µs 5.35 µs 0.9766 5 KB
-Entry 1,951.2 µs 5.02 µs 4.19 µs - 12 KB
+Entry 1,925.8 µs 13.24 µs 11.74 µs - 12 KB

- Using dotnet/maui/main before these changes
+ Using these changes

I tried a few times, and I'm not sure I'm able to see a measurable
difference launching Maui.Controls.Sample.SingleProject.csproj over
and over. But this change seems low risk, worth adding in.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the WinUI, Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

Does this PR touch anything that might affect accessibility?

No

@Redth Redth enabled auto-merge (squash) December 17, 2021 16:12
Previously we were calculating:

    public static bool HasRtlSupport(this Context self)
    {
        if (self == null)
            return false;

        return (self.ApplicationInfo?.Flags & AApplicationInfoFlags.SupportsRtl) == AApplicationInfoFlags.SupportsRtl;
    }

Multiple times for every `Label`, `Entry`, or related controls.
`Context.ApplicationInfo` pulls values from the `AndroidManifest.xml`
for the entire Android application, so this isn't a value that would
not change after startup -- or for different controls.

https://developer.android.com/reference/android/content/pm/ApplicationInfo#flags
https://developer.android.com/reference/android/content/pm/ApplicationInfo#FLAG_SUPPORTS_RTL

Let's move this method to be a `static readonly` field in a new static
class. It will lazily be evaluated when the new class is accessed, and
only once per application run.

~~ Results ~~

Running `ViewHandlerBenchmark` on a Pixel 5 device:

    > .\bin\dotnet\dotnet build .\src\Core\tests\Benchmarks.Droid\Benchmarks.Droid.csproj -t:Benchmark -c Release

| Method |       Mean |    Error |   StdDev |  Gen 0 | Allocated |
|------- |-----------:|---------:|---------:|-------:|----------:|
| -Label |   527.6 µs |  4.89 µs |  4.57 µs | 0.9766 |      5 KB |
| +Label |   494.9 µs |  5.72 µs |  5.35 µs | 0.9766 |      5 KB |
| -Entry | 1,951.2 µs |  5.02 µs |  4.19 µs |      - |     12 KB |
| +Entry | 1,925.8 µs | 13.24 µs | 11.74 µs |      - |     12 KB |

- Using dotnet/maui/main before these changes
+ Using these changes

I tried a few times, and I'm not sure I'm able to see a measurable
difference launching `Maui.Controls.Sample.SingleProject.csproj` over
and over. But this change seems low risk, worth adding in.
@Redth Redth merged commit a3ee3d8 into dotnet:main Dec 17, 2021
@jonathanpeppers jonathanpeppers deleted the hasrtlsupport branch December 20, 2021 15:10
@samhouts samhouts added legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-editor Editor area-controls-entry Entry area-controls-picker Picker platform/android 🤖 labels Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added the fixed-in-6.0.200-preview.12 Look for this fix in 6.0.200-preview.12! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-editor Editor area-controls-entry Entry area-controls-picker Picker area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-6.0.200-preview.12 Look for this fix in 6.0.200-preview.12! platform/android 🤖
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants