Skip to content

Commit

Permalink
SONARJAVA-4841 Add S6898: Avoid high frame rates on mobile (#4652)
Browse files Browse the repository at this point in the history
  • Loading branch information
johann-beleites-sonarsource committed Feb 19, 2024
1 parent d3a893e commit 5c53492
Show file tree
Hide file tree
Showing 8 changed files with 234 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ public class Surface {
public void release() {
// mock implementation
}

public void setFrameRate(float frameRate, int compatibility, int changeFrameRateStrategy) {
}

public void setFrameRate(float frameRate, int compatibility) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package android.view;

public class SurfaceControl {
public SurfaceControl.Transaction setFrameRate(SurfaceControl sc, float frameRate, int compatibility) {
return new Transaction();
}

public SurfaceControl.Transaction setFrameRate(SurfaceControl sc, float frameRate, int compatibility, int changeFrameRateStrategy) {
return new Transaction();
}

public static class Transaction {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package checks;

import android.view.Surface;
import android.view.SurfaceControl;

public class AvoidHighFrameratesOnMobileCheckSample {

private static final int HIGH_THRESHOLD = 120;
private int HIGH_THRESHOLD_NON_FINAL = 120;

void noncompliant(Surface surface, SurfaceControl surfaceControl) {
surface.setFrameRate(120, 0, 0); // Noncompliant [[sc=26;ec=29]] {{Avoid setting high frame rates higher than 60 on mobile devices.}}
surface.setFrameRate(120, 0); // Noncompliant
surfaceControl.setFrameRate(surfaceControl, 120, 0, 0); // Noncompliant
surfaceControl.setFrameRate(surfaceControl, 120, 0); // Noncompliant

surface.setFrameRate(61, 0, 0); // Noncompliant
surface.setFrameRate(HIGH_THRESHOLD, 0); // Noncompliant
}

void compliant(Surface surface, SurfaceControl surfaceControl) {
surface.setFrameRate(60, 0, 0);
surface.setFrameRate(60, 0);
surfaceControl.setFrameRate(surfaceControl, 60, 0, 0);
surfaceControl.setFrameRate(surfaceControl, 60, 0);

surface.setFrameRate(1, 0, 0);
surface.setFrameRate(HIGH_THRESHOLD_NON_FINAL, 0); // Compliant FN, as we don't currently resolve non-final non-static fields.
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* SonarQube Java
* Copyright (C) 2012-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.checks;

import java.util.List;
import java.util.Map;
import org.sonar.check.Rule;
import org.sonar.java.model.ExpressionUtils;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.Tree;

@Rule(key = "S6898")
public class AvoidHighFrameratesOnMobileCheck extends IssuableSubscriptionVisitor {

private static final int DEFAULT_THRESHOLD = 60;
private static final Map<MethodMatchers, Integer> FRAME_RATE_SETTERS = Map.of(
MethodMatchers.create().ofTypes("android.view.Surface").names("setFrameRate").withAnyParameters().build(), 0,
MethodMatchers.create().ofTypes("android.view.SurfaceControl").names("setFrameRate").withAnyParameters().build(), 1);

@Override
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.METHOD_INVOCATION);
}

@Override
public void visitNode(Tree tree) {
var mit = (MethodInvocationTree) tree;
FRAME_RATE_SETTERS.entrySet().stream().filter(e -> e.getKey().matches(mit)).findFirst().ifPresent(e -> {
var frameRateArg = mit.arguments().get(e.getValue());
var frameRateArgVal = ExpressionUtils.resolveAsConstant(frameRateArg);

if (frameRateArgVal instanceof Number frameRateNumber && frameRateNumber.intValue() > DEFAULT_THRESHOLD) {
reportIssue(frameRateArg, "Avoid setting high frame rates higher than " + DEFAULT_THRESHOLD + " on mobile devices.");
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* SonarQube Java
* Copyright (C) 2012-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;
import org.sonar.java.checks.verifier.TestUtils;

class AvoidHighFrameratesOnMobileCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile(TestUtils.mainCodeSourcesPath("checks/AvoidHighFrameratesOnMobileCheckSample.java"))
.withCheck(new AvoidHighFrameratesOnMobileCheck())
.verifyIssues();
}

@Test
void test_no_semantics() {
CheckVerifier.newVerifier()
.onFile(TestUtils.mainCodeSourcesPath("checks/AvoidHighFrameratesOnMobileCheckSample.java"))
.withCheck(new AvoidHighFrameratesOnMobileCheck())
.withoutSemantic()
.verifyNoIssues();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.sonar.java.checks.AssertsOnParametersOfPublicMethodCheck;
import org.sonar.java.checks.AssignmentInSubExpressionCheck;
import org.sonar.java.checks.AtLeastOneConstructorCheck;
import org.sonar.java.checks.AvoidHighFrameratesOnMobileCheck;
import org.sonar.java.checks.BasicAuthCheck;
import org.sonar.java.checks.BigDecimalDoubleConstructorCheck;
import org.sonar.java.checks.BluetoothLowPowerModeCheck;
Expand Down Expand Up @@ -724,6 +725,7 @@ public final class CheckList {
AuthorizationsStrongDecisionsCheck.class,
AutowiredOnConstructorWhenMultipleConstructorsCheck.class,
AutowiredOnMultipleConstructorsCheck.class,
AvoidHighFrameratesOnMobileCheck.class,
AvoidQualifierOnBeanMethodsCheck.class,
AwsConsumerBuilderUsageCheck.class,
AwsCredentialsShouldBeSetExplicitlyCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<p>The <em>Frame Rate API</em> allows applications to communicate their desired frame rate to the <em>Android platform</em> to enhance the user
experience. The API is useful since many devices now offer varying refresh rates like 60Hz, 90Hz, or 120Hz.</p>
<h2>Why is this an issue?</h2>
<p>Standard applications don’t require a display refresh rate above 60Hz, hence it is advisable to avoid higher frequencies to avoid unnecessary
energy consumption.</p>
<p>The rule flags an issue when <code>setFrameRate()</code> is invoked with a frameRate higher than 60Hz for <code>android.view.Surface</code> and
<code>android.view.SurfaceControl.Transaction</code>.</p>
<p>It’s important to note that the scheduler considers several factors when determining the display refresh rate. Therefore, using
<code>setFrameRate()</code> doesn’t guarantee your app will achieve the requested frame rate.</p>
<h3>What is the potential impact?</h3>
<ul>
<li> <em>Usability</em>: the device may run out of battery faster than expected. </li>
<li> <em>Sustainability</em>: the extra battery usage has a negative impact on the environment. </li>
</ul>
<h2>How to fix it</h2>
<p>Use a frame rate of maximum 60Hz, unless you have a strong reason to used higher rates. Valid exceptions are <em>gaming apps</em>, especially those
with fast-paced action or high-quality graphics, or <em>AR/VR apps</em>.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public class MainActivity extends AppCompatActivity {

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);

SurfaceView surfaceView = findViewById(R.id.my_surface_view);
Surface surface = surfaceView.getHolder().getSurface();

surface.setFrameRate(90.0f, Surface.FRAME_RATE_COMPATIBILITY_FIXED_SOURCE); // Noncompliant
}
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public class MainActivity extends AppCompatActivity {

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);

SurfaceView surfaceView = findViewById(R.id.my_surface_view);
Surface surface = surfaceView.getHolder().getSurface();

surface.setFrameRate(60.0f, Surface.FRAME_RATE_COMPATIBILITY_FIXED_SOURCE); // Compliant
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://developer.android.com/media/optimize/performance/frame-rate">Android for Developers: Frame Rate</a> </li>
<li> <a
href="https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/FrameRate.html#//apple_ref/doc/uid/TP40016642-CH23-SW1">Developer Apple - Frame Rate</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"title": "High frame rates should not be used",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"android",
"sustainability"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-6898",
"sqKey": "S6898",
"scope": "Main",
"quickfix": "unknown",
"code": {
"impacts": {
"MAINTAINABILITY": "LOW"
},
"attribute": "EFFICIENT"
}
}

0 comments on commit 5c53492

Please sign in to comment.