Skip to content

Commit

Permalink
core(layout-shift-elements): aggregate all remaining elements (#15593)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Dec 8, 2023
1 parent 470505e commit 2338efc
Show file tree
Hide file tree
Showing 9 changed files with 369 additions and 251 deletions.
33 changes: 22 additions & 11 deletions core/audits/layout-shift-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,28 @@ class LayoutShiftElements extends Audit {
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const clsElements = artifacts.TraceElements
.filter(element => element.traceEventType === 'layout-shift');
const {cumulativeLayoutShift: clsSavings, impactByNodeId} =
await CumulativeLayoutShiftComputed.request(artifacts.traces[Audit.DEFAULT_PASS], context);

const clsElementData = clsElements.map(element => {
return {
/** @type {Array<{node: LH.Audit.Details.ItemValue, score?: number}>} */
const clsElementData = artifacts.TraceElements
.filter(element => element.traceEventType === 'layout-shift')
.map(element => ({
node: Audit.makeNodeItem(element.node),
score: element.score,
};
});
}));

if (clsElementData.length < impactByNodeId.size) {
const elementDataImpact = clsElementData.reduce((sum, {score}) => sum += score || 0, 0);
const remainingImpact = Math.max(clsSavings - elementDataImpact, 0);
clsElementData.push({
node: {
type: 'code',
value: str_(i18n.UIStrings.otherResourceType),
},
score: remainingImpact,
});
}

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand All @@ -58,15 +71,13 @@ class LayoutShiftElements extends Audit {
];

const details = Audit.makeTableDetails(headings, clsElementData);

let displayValue;
if (clsElementData.length > 0) {
if (impactByNodeId.size > 0) {
displayValue = str_(i18n.UIStrings.displayValueElementsFound,
{nodeCount: clsElementData.length});
{nodeCount: impactByNodeId.size});
}

const {cumulativeLayoutShift: clsSavings} =
await CumulativeLayoutShiftComputed.request(artifacts.traces[Audit.DEFAULT_PASS], context);

const passed = clsSavings <= CumulativeLayoutShift.defaultOptions.p10;

return {
Expand Down
6 changes: 5 additions & 1 deletion core/audits/metrics/cumulative-layout-shift.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ class CumulativeLayoutShift extends Audit {
*/
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const {cumulativeLayoutShift, ...rest} = await ComputedCLS.request(trace, context);

// impactByNodeId is unused but we don't want it on debug data
// eslint-disable-next-line no-unused-vars
const {cumulativeLayoutShift, impactByNodeId, ...rest} =
await ComputedCLS.request(trace, context);

/** @type {LH.Audit.Details.DebugData} */
const details = {
Expand Down
48 changes: 47 additions & 1 deletion core/computed/metrics/cumulative-layout-shift.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import {makeComputedArtifact} from '../computed-artifact.js';
import {ProcessedTrace} from '../processed-trace.js';
import * as RectHelpers from '../../lib/rect-helpers.js';

/** @typedef {{ts: number, isMainFrame: boolean, weightedScore: number, impactedNodes?: LH.Artifacts.TraceImpactedNode[]}} LayoutShiftEvent */

Expand Down Expand Up @@ -72,6 +73,49 @@ class CumulativeLayoutShift {
return layoutShiftEvents;
}

/**
* Each layout shift event has a 'score' which is the amount added to the CLS as a result of the given shift(s).
* We calculate the score per element by taking the 'score' of each layout shift event and
* distributing it between all the nodes that were shifted, proportianal to the impact region of
* each shifted element.
*
* @param {LayoutShiftEvent[]} layoutShiftEvents
* @return {Map<number, number>}
*/
static getImpactByNodeId(layoutShiftEvents) {
/** @type {Map<number, number>} */
const impactByNodeId = new Map();

for (const event of layoutShiftEvents) {
if (!event.impactedNodes) continue;

let totalAreaOfImpact = 0;
/** @type {Map<number, number>} */
const pixelsMovedPerNode = new Map();

for (const node of event.impactedNodes) {
if (!node.node_id || !node.old_rect || !node.new_rect) continue;

const oldRect = RectHelpers.traceRectToLHRect(node.old_rect);
const newRect = RectHelpers.traceRectToLHRect(node.new_rect);
const areaOfImpact = RectHelpers.getRectArea(oldRect) +
RectHelpers.getRectArea(newRect) -
RectHelpers.getRectOverlapArea(oldRect, newRect);

pixelsMovedPerNode.set(node.node_id, areaOfImpact);
totalAreaOfImpact += areaOfImpact;
}

for (const [nodeId, pixelsMoved] of pixelsMovedPerNode.entries()) {
let clsContribution = impactByNodeId.get(nodeId) || 0;
clsContribution += (pixelsMoved / totalAreaOfImpact) * event.weightedScore;
impactByNodeId.set(nodeId, clsContribution);
}
}

return impactByNodeId;
}

/**
* Calculates cumulative layout shifts per cluster (session) of LayoutShift
* events -- where a new cluster is created when there's a gap of more than
Expand Down Expand Up @@ -104,18 +148,20 @@ class CumulativeLayoutShift {
/**
* @param {LH.Trace} trace
* @param {LH.Artifacts.ComputedContext} context
* @return {Promise<{cumulativeLayoutShift: number, cumulativeLayoutShiftMainFrame: number}>}
* @return {Promise<{cumulativeLayoutShift: number, cumulativeLayoutShiftMainFrame: number, impactByNodeId: Map<number, number>}>}
*/
static async compute_(trace, context) {
const processedTrace = await ProcessedTrace.request(trace, context);

const allFrameShiftEvents =
CumulativeLayoutShift.getLayoutShiftEvents(processedTrace);
const impactByNodeId = CumulativeLayoutShift.getImpactByNodeId(allFrameShiftEvents);
const mainFrameShiftEvents = allFrameShiftEvents.filter(e => e.isMainFrame);

return {
cumulativeLayoutShift: CumulativeLayoutShift.calculate(allFrameShiftEvents),
cumulativeLayoutShiftMainFrame: CumulativeLayoutShift.calculate(mainFrameShiftEvents),
impactByNodeId,
};
}
}
Expand Down
88 changes: 19 additions & 69 deletions core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import BaseGatherer from '../base-gatherer.js';
import {resolveNodeIdToObjectId} from '../driver/dom.js';
import {pageFunctions} from '../../lib/page-functions.js';
import * as RectHelpers from '../../lib/rect-helpers.js';
import {Sentry} from '../../lib/sentry.js';
import Trace from './trace.js';
import {ProcessedTrace} from '../../computed/processed-trace.js';
Expand All @@ -27,6 +26,8 @@ import {ExecutionContext} from '../driver/execution-context.js';

/** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[], type?: string}} TraceElementData */

const MAX_LAYOUT_SHIFT_ELEMENTS = 15;

/**
* @this {HTMLElement}
*/
Expand Down Expand Up @@ -63,75 +64,24 @@ class TraceElements extends BaseGatherer {
}

/**
* @param {Array<number>} rect
* @return {LH.Artifacts.Rect}
*/
static traceRectToLHRect(rect) {
const rectArgs = {
x: rect[0],
y: rect[1],
width: rect[2],
height: rect[3],
};
return RectHelpers.addRectTopAndBottom(rectArgs);
}

/**
* This function finds the top (up to 5) elements that contribute to the CLS score of the page.
* Each layout shift event has a 'score' which is the amount added to the CLS as a result of the given shift(s).
* We calculate the score per element by taking the 'score' of each layout shift event and
* distributing it between all the nodes that were shifted, proportianal to the impact region of
* each shifted element.
* @param {LH.Artifacts.ProcessedTrace} processedTrace
* @return {Array<TraceElementData>}
* This function finds the top (up to 15) elements that contribute to the CLS score of the page.
*
* @param {LH.Trace} trace
* @param {LH.Gatherer.Context} context
* @return {Promise<Array<TraceElementData>>}
*/
static getTopLayoutShiftElements(processedTrace) {
/** @type {Map<number, number>} */
const clsPerNode = new Map();
const shiftEvents = CumulativeLayoutShift.getLayoutShiftEvents(processedTrace);

shiftEvents.forEach((event) => {
if (!event || !event.impactedNodes) {
return;
}

let totalAreaOfImpact = 0;
/** @type {Map<number, number>} */
const pixelsMovedPerNode = new Map();

event.impactedNodes.forEach(node => {
if (!node.node_id || !node.old_rect || !node.new_rect) {
return;
}

const oldRect = TraceElements.traceRectToLHRect(node.old_rect);
const newRect = TraceElements.traceRectToLHRect(node.new_rect);
const areaOfImpact = RectHelpers.getRectArea(oldRect) +
RectHelpers.getRectArea(newRect) -
RectHelpers.getRectOverlapArea(oldRect, newRect);

pixelsMovedPerNode.set(node.node_id, areaOfImpact);
totalAreaOfImpact += areaOfImpact;
static async getTopLayoutShiftElements(trace, context) {
const {impactByNodeId} = await CumulativeLayoutShift.request(trace, context);

return [...impactByNodeId.entries()]
.sort((a, b) => b[1] - a[1])
.slice(0, MAX_LAYOUT_SHIFT_ELEMENTS)
.map(([nodeId, clsContribution]) => {
return {
nodeId: nodeId,
score: clsContribution,
};
});

for (const [nodeId, pixelsMoved] of pixelsMovedPerNode.entries()) {
let clsContribution = clsPerNode.get(nodeId) || 0;
clsContribution += (pixelsMoved / totalAreaOfImpact) * event.weightedScore;
clsPerNode.set(nodeId, clsContribution);
}
});

const topFive = [...clsPerNode.entries()]
.sort((a, b) => b[1] - a[1])
.slice(0, 5)
.map(([nodeId, clsContribution]) => {
return {
nodeId: nodeId,
score: clsContribution,
};
});

return topFive;
}

/**
Expand Down Expand Up @@ -265,7 +215,7 @@ class TraceElements extends BaseGatherer {
const {mainThreadEvents} = processedTrace;

const lcpNodeData = await TraceElements.getLcpElement(trace, context);
const clsNodeData = TraceElements.getTopLayoutShiftElements(processedTrace);
const clsNodeData = await TraceElements.getTopLayoutShiftElements(trace, context);
const animatedElementData = await this.getAnimatedElements(mainThreadEvents);
const responsivenessElementData = await TraceElements.getResponsivenessElement(trace, context);

Expand Down
15 changes: 15 additions & 0 deletions core/lib/rect-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,20 @@ function allRectsContainedWithinEachOther(rectListA, rectListB) {
return true;
}

/**
* @param {Array<number>} rect
* @return {LH.Artifacts.Rect}
*/
function traceRectToLHRect(rect) {
const rectArgs = {
x: rect[0],
y: rect[1],
width: rect[2],
height: rect[3],
};
return addRectTopAndBottom(rectArgs);
}

export {
rectContainsPoint,
rectContains,
Expand All @@ -248,4 +262,5 @@ export {
allRectsContainedWithinEachOther,
filterOutRectsContainedByOthers,
filterOutTinyRects,
traceRectToLHRect,
};
Loading

0 comments on commit 2338efc

Please sign in to comment.