Skip to content

Commit

Permalink
Unreviewed, reverting 254133@main.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260215

Gave incorrect results when running MotionMark from the main page on 120Hz devices

Reverted changeset:

"Support non-60 frame rates in MotionMark"
https://bugs.webkit.org/show_bug.cgi?id=244656
https://commits.webkit.org/254133@main

Canonical link: https://commits.webkit.org/266923@main
  • Loading branch information
webkit-commit-queue authored and smfr committed Aug 15, 2023
1 parent fed29de commit 75aa42a
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 110 deletions.
1 change: 0 additions & 1 deletion PerformanceTests/MotionMark/about.html
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ <h1>About MotionMark <span class="version"></span></h1>
<h3>Version log</h3>

<ul id="log">
<li><strong>1.3</strong>: Add support for non-60Hz <code>requestAnimationFrame</code> rates to the <a href="developer.html">developer settings</a>.</li>
<li><strong>1.2</strong>: Fix <a href="https://bugs.webkit.org/show_bug.cgi?id=220847">bug</a>, <a href="https://bugs.webkit.org/show_bug.cgi?id=221075">bug</a>, and <a href="https://bugs.webkit.org/show_bug.cgi?id=219984">bug</a> to reduce test variance and sensitivity to individual long frames.</li>
<li><strong>1.1.1</strong>: Fix <a href="https://bugs.webkit.org/show_bug.cgi?id=210640">bug</a> in the calculation of timestamps used for animation during warm up phase of tests.</li>
<li><a href="https://webkit.org/blog/8434/motionmark-1-1/"><strong>1.1</strong></a>: Update Multiply test to increase max capacity and expand methods for hiding elements. Update Leaves test to use range of sizes and opacity.</li>
Expand Down
13 changes: 2 additions & 11 deletions PerformanceTests/MotionMark/developer.html
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ <h3>Adjusting the test complexity:</h3>
</ul>
</li>
<li>
<label>System frame rate: <input type="number" id="system-frame-rate" value="60"> FPS</label><br>
<label>Target frame rate: <input type="number" id="frame-rate" value="50"> FPS</label><br>
(Guide: should be about 5/6th of the system frame rate)
<label>Target frame rate: <input type="number" id="frame-rate" value="50"> FPS</label>
</li>
<li>
<h3>Time measurement method:</h3>
Expand All @@ -115,14 +113,7 @@ <h3>Time measurement method:</h3>
</form>
</div>
</div>
<p>
For accurate results, please take the browser window full screen, or rotate the device to landscape orientation. Also,
ensure that the target frame rate matches your system frame rate. Results cannot be compared between devices that
use different frame rates.
</p>
<p id="frame-rate-detection">
Attempting to detect system frame rate: <span>0</span> FPS (in progress).
</p>
<p>For accurate results, please take the browser window full screen, or rotate the device to landscape orientation.</p>
<div class="start-benchmark">
<p class="hidden">Please rotate the device to orientation before starting.</p>
<button id="run-benchmark" onclick="benchmarkController.startBenchmark()">Run benchmark</button>
Expand Down
12 changes: 5 additions & 7 deletions PerformanceTests/MotionMark/resources/debug-runner/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ Utilities.extendObject(window.benchmarkController, {
samplesWithProperties[seriesName] = series.toArray();
})

this._targetFrameRate = options["frame-rate"] || 60;

this.createTimeGraph(testResult, samplesWithProperties[Strings.json.controller], testData[Strings.json.marks], testData[Strings.json.controller], options, margins, size);
this.onTimeGraphOptionsChanged();

Expand Down Expand Up @@ -129,14 +127,14 @@ Utilities.extendObject(window.benchmarkController, {
.domain([xMin, xMax]);
var yScale = d3.scale.linear()
.range([size.height, 0])
.domain([1000/(this._targetFrameRate/3), 1000/this._targetFrameRate]);
.domain([1000/20, 1000/60]);

var xAxis = d3.svg.axis()
.scale(xScale)
.orient("bottom");
var yAxis = d3.svg.axis()
.scale(yScale)
.tickValues([1000/20, 1000/25, 1000/30, 1000/35, 1000/40, 1000/45, 1000/50, 1000/55, 1000/60, 1000/90, 1000/120])
.tickValues([1000/20, 1000/25, 1000/30, 1000/35, 1000/40, 1000/45, 1000/50, 1000/55, 1000/60])
.tickFormat(function(d) { return (1000 / d).toFixed(0); })
.orient("left");

Expand Down Expand Up @@ -286,7 +284,7 @@ Utilities.extendObject(window.benchmarkController, {
.domain([0, complexityMax]);
var yRight = d3.scale.linear()
.range([size.height, 0])
.domain([1000/(this._targetFrameRate/3), 1000/this._targetFrameRate]);
.domain([1000/20, 1000/60]);

// Axes
var xAxis = d3.svg.axis()
Expand All @@ -298,7 +296,7 @@ Utilities.extendObject(window.benchmarkController, {
.orient("left");
var yAxisRight = d3.svg.axis()
.scale(yRight)
.tickValues([1000/20, 1000/25, 1000/30, 1000/35, 1000/40, 1000/45, 1000/50, 1000/55, 1000/60, 1000/90, 1000/120])
.tickValues([1000/20, 1000/25, 1000/30, 1000/35, 1000/40, 1000/45, 1000/50, 1000/55, 1000/60])
.tickFormat(function(d) { return (1000/d).toFixed(0); })
.orient("right");

Expand Down Expand Up @@ -625,6 +623,6 @@ Utilities.extendObject(window.benchmarkController, {
}
}

sectionsManager.setSectionScore("test-graph", score, mean, this._targetFrameRate);
sectionsManager.setSectionScore("test-graph", score, mean);
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,6 @@ label.tree-label {
color: hsl(11, 100%, 66%);
}

#frame-rate-detection span {
color: red;
}

@media screen and (max-device-width: 414px),
screen and (max-device-height: 414px) and (orientation: landscape) {
#intro .body > div:first-of-type {
Expand Down
47 changes: 1 addition & 46 deletions PerformanceTests/MotionMark/resources/debug-runner/motionmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,6 @@ Utilities.extendObject(window.benchmarkController, {
suitesManager.updateUIFromLocalStorage();
suitesManager.updateEditsElementsState();

benchmarkController.detectSystemFrameRate();

var dropTarget = document.getElementById("drop-target");
function stopEvent(e) {
e.stopPropagation();
Expand Down Expand Up @@ -666,9 +664,8 @@ Utilities.extendObject(window.benchmarkController, {
var score = dashboard.score;
var confidence = ((dashboard.scoreLowerBound / score - 1) * 100).toFixed(2) +
"% / +" + ((dashboard.scoreUpperBound / score - 1) * 100).toFixed(2) + "%";
var fps = dashboard._systemFrameRate;
sectionsManager.setSectionVersion("results", dashboard.version);
sectionsManager.setSectionScore("results", score.toFixed(2), confidence, fps);
sectionsManager.setSectionScore("results", score.toFixed(2), confidence);
sectionsManager.populateTable("results-header", Headers.testName, dashboard);
sectionsManager.populateTable("results-score", Headers.score, dashboard);
sectionsManager.populateTable("results-data", Headers.details, dashboard);
Expand All @@ -682,47 +679,5 @@ Utilities.extendObject(window.benchmarkController, {
sectionsManager.setSectionHeader("test-graph", testName);
sectionsManager.showSection("test-graph", true);
this.updateGraphData(testResult, testData, benchmarkRunnerClient.results.options);
},
detectSystemFrameRate: function()
{
let last = 0;
let average = 0;
let count = 0;

const finish = function()
{
const commonFrameRates = [15, 30, 45, 60, 90, 120, 144];
const distanceFromFrameRates = commonFrameRates.map(rate => {
return Math.abs(Math.round(rate - average));
});
let shortestDistance = Number.MAX_VALUE;
let targetFrameRate = undefined;
for (let i = 0; i < commonFrameRates.length; i++) {
if (distanceFromFrameRates[i] < shortestDistance) {
targetFrameRate = commonFrameRates[i];
shortestDistance = distanceFromFrameRates[i];
}
}
targetFrameRate = targetFrameRate || 60;
document.getElementById("frame-rate-detection").textContent = `Detected system frame rate as ${targetFrameRate} FPS`;
document.getElementById("system-frame-rate").value = targetFrameRate;
document.getElementById("frame-rate").value = Math.round(targetFrameRate * 5 / 6);
}

const tick = function(timestamp)
{
average -= average / 30;
average += 1000. / (timestamp - last) / 30;
document.querySelector("#frame-rate-detection span").textContent = Math.round(average);
last = timestamp;
count++;
if (count < 300)
requestAnimationFrame(tick);
else
finish();
}

requestAnimationFrame(tick);
}

});
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ section .body {
section .body p {
margin: 1em 0;
line-height: 1.5em;
max-width: 60vw;

-webkit-user-select: text;
cursor: text;
Expand Down
16 changes: 4 additions & 12 deletions PerformanceTests/MotionMark/resources/runner/motionmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
this._options = options;
this._results = null;
this._version = version;
this._targetFrameRate = options["frame-rate"] || 60;
this._systemFrameRate = options["system-frame-rate"] || 60;
if (testData) {
this._iterationsSamplers = testData;
this._processData();
Expand Down Expand Up @@ -85,7 +83,6 @@
}, this);

this._results[Strings.json.version] = this._version;
this._results[Strings.json.fps] = this._targetFrameRate;
this._results[Strings.json.score] = Statistics.sampleMean(iterationsScores.length, iterationsScores.reduce(function(a, b) { return a + b; }));
this._results[Strings.json.scoreLowerBound] = this._results[Strings.json.results.iterations][0][Strings.json.scoreLowerBound];
this._results[Strings.json.scoreUpperBound] = this._results[Strings.json.results.iterations][0][Strings.json.scoreUpperBound];
Expand All @@ -112,7 +109,7 @@

var complexityIndex = series.fieldMap[Strings.json.complexity];
var frameLengthIndex = series.fieldMap[Strings.json.frameLength];
var regressionOptions = { desiredFrameLength: 1000/this._targetFrameRate };
var regressionOptions = { desiredFrameLength: 1000/60 };
if (profile)
regressionOptions.preferredProfile = profile;
return {
Expand Down Expand Up @@ -168,8 +165,6 @@
result[Strings.json.complexity][Strings.json.complexity] = calculation.complexity;
result[Strings.json.complexity][Strings.json.measurements.stdev] = Math.sqrt(calculation.error / samples[Strings.json.complexity].length);

result[Strings.json.fps] = data.targetFPS || 60;

if (isRampController) {
var timeComplexity = new Experiment;
data[Strings.json.controller].forEach(function(regression) {
Expand Down Expand Up @@ -447,10 +442,9 @@ window.sectionsManager =
document.querySelector("#" + sectionIdentifier + " .version").textContent = version;
},

setSectionScore: function(sectionIdentifier, score, confidence, fps)
setSectionScore: function(sectionIdentifier, score, confidence)
{
if (fps && score)
document.querySelector("#" + sectionIdentifier + " .score").textContent = `${score} @ ${fps}fps`;
document.querySelector("#" + sectionIdentifier + " .score").textContent = score;
if (confidence)
document.querySelector("#" + sectionIdentifier + " .confidence").textContent = confidence;
},
Expand Down Expand Up @@ -563,9 +557,8 @@ window.benchmarkController = {
var dashboard = benchmarkRunnerClient.results;
var score = dashboard.score;
var confidence = "±" + (Statistics.largestDeviationPercentage(dashboard.scoreLowerBound, score, dashboard.scoreUpperBound) * 100).toFixed(2) + "%";
var fps = dashboard._systemFrameRate;
sectionsManager.setSectionVersion("results", dashboard.version);
sectionsManager.setSectionScore("results", score.toFixed(2), confidence, fps);
sectionsManager.setSectionScore("results", score.toFixed(2), confidence);
sectionsManager.populateTable("results-header", Headers.testName, dashboard);
sectionsManager.populateTable("results-score", Headers.score, dashboard);
sectionsManager.populateTable("results-data", Headers.details, dashboard);
Expand Down Expand Up @@ -665,4 +658,3 @@ window.benchmarkController = {
};

window.addEventListener("load", function() { benchmarkController.initialize(); });

3 changes: 1 addition & 2 deletions PerformanceTests/MotionMark/resources/statistics.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ Experiment.defaults =
Regression = Utilities.createClass(
function(samples, getComplexity, getFrameLength, startIndex, endIndex, options)
{
var targetFrameRate = options["frame-rate"] || 60;
var desiredFrameLength = options.desiredFrameLength || 1000/targetFrameRate;
var desiredFrameLength = options.desiredFrameLength || 1000/60;
var bestProfile;

if (!options.preferredProfile || options.preferredProfile == Strings.json.profiles.slope) {
Expand Down
3 changes: 1 addition & 2 deletions PerformanceTests/MotionMark/resources/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
var Strings = {
version: "1.3",
version: "1.2",
text: {
testName: "Test Name",
score: "Score",
Expand All @@ -49,7 +49,6 @@ var Strings = {
score: "score",
scoreLowerBound: "scoreLowerBound",
scoreUpperBound: "scoreUpperBound",
fps: "fps",
bootstrap: "bootstrap",
measurements: {
average: "average",
Expand Down
Loading

0 comments on commit 75aa42a

Please sign in to comment.