Screenshot: stop repeating the same screenshot over and over when it doesn't change

[julienw]

In this patch we want to do an experiment, it's not sure we'll land this in the end.
This experiment is that we don't want that we repeat the screenshot in the screenshot track, if it doesn't change.

The current logic is at

profiler/src/components/timeline/TrackScreenshots.js

Lines 273 to 333 in 0ecc660

	render() {
	const {
	thread,
	width: outerContainerWidth,
	rangeStart,
	rangeEnd,
	screenshots,
	trackHeight,
	} = this.props;
	if (screenshots.length === 0) {
	return null;
	}
	const images = [];
	const rangeLength = rangeEnd - rangeStart;
	const imageContainerWidth = trackHeight * 0.75;
	const timeToPixel = time =>
	(outerContainerWidth * (time - rangeStart)) / rangeLength;
	const leftmostPixel = Math.max(timeToPixel(screenshots[0].start), 0);
	let screenshotIndex = 0;
	for (
	let left = leftmostPixel;
	left < outerContainerWidth;
	left += imageContainerWidth
	) {
	// Try to find the next screenshot to fit in, or re-use the existing one.
	for (let i = screenshotIndex; i < screenshots.length; i++) {
	if (timeToPixel(screenshots[i].start) <= left) {
	screenshotIndex = i;
	} else {
	break;
	}
	}
	// Coerce the payload into a screenshot one.
	const payload: ScreenshotPayload = (screenshots[screenshotIndex]
	.data: any);
	const { url: urlStringIndex, windowWidth, windowHeight } = payload;
	const scaledImageWidth = (trackHeight * windowWidth) / windowHeight;
	images.push(
	<div
	className="timelineTrackScreenshotImgContainer"
	style={{ left, width: imageContainerWidth }}
	key={left}
	>
	{/* The following image is centered and cropped by the outer container. */}
	<img
	className="timelineTrackScreenshotImg"
	src={thread.stringTable.getString(urlStringIndex)}
	style={{
	width: scaledImageWidth,
	height: trackHeight,
	}}
	/>
	</div>
	);
	}
	return images;
	}
	}

What it does is that for each pixel we try to find the right screenshot. Especially here:

profiler/src/components/timeline/TrackScreenshots.js

Lines 301 to 307 in 0ecc660

	for (let i = screenshotIndex; i < screenshots.length; i++) {
	if (timeToPixel(screenshots[i].start) <= left) {
	screenshotIndex = i;
	} else {
	break;
	}
	}

We see that if don't find a screenshot, by breaking we reuse the previous data.

We also see that there are 2 nested loops: the outer loop loops over the pixel "left" information, and the inner loops over the screenshots.

Instead I think we'll want to invert the logic: the outer loop looping over the screenshots, and some logic inside to compute the "left" information. Especially take care that we don't overlap images, so when looping over the screenshots you'll need to skip them until we have some space. When finally we have some space we'll need to display the last screenshot.

Maybe that's not the easiest solution, and instead we should just change something at the break in the existing code. So this will need some research.

Here are some profile links containing screenshots:

• https://profiler.firefox.com/public/404ff08ee5f412f355264601f12e5feff0ebaee6/marker-chart/?globalTrackOrder=6-1-2-3-4-0-5&thread=6&v=3
• https://profiler.firefox.com/public/d4e0b257c771dd7d4a7a6bd6c8c8cbb2d5dad40c/calltree/?globalTrackOrder=10-11-12-13-0-1-2-3-4-5-6-7-8-9&hiddenGlobalTracks=1-2-3-4-5-6-7-9&thread=9&v=3
• https://profiler.firefox.com/public/36d6d200bb8ec4eff5a90044dc066063703edf6a/calltree/?globalTrackOrder=4-5-0-1-2-3&hiddenGlobalTracks=1-2-3&thread=0&v=3

┆Issue is synchronized with this Jira Task

[raymond-sikpojie]

Can I give this a try? @julienw

[lorelajano]

Can I work on this?

[julienw]

Hey @raymond-sikpojie, yes sure!
@lorelajano as you see, this issue is taken already, thanks for your understanding.

[raymond-sikpojie]

Hey @julienw. I tried but I really don't know how to go about this one. Can I be unassigned from this issue? Thanks

[ghost]

@julienw can i give this a go?

[canova]

Sure @karenefereyan, let us know if you need anything!

[julienw]

hey @karenefereyan, is that still something you'd like to work on? It's totally OK if you don't, but please tell us so that we can reset the status of this issue. Thanks!

[ghost]

hey @karenefereyan, is that still something you'd like to work on? It's totally OK if you don't, but please tell us so that we can reset the status of this issue. Thanks!

@julienw please reassign it to someone else as I'm having some technical challenges currently

[julienw]

No worries, thanks for telling us :-)

[Bucky25]

@julienw i would like to work on it.

[Bucky25]

what exactly is the task is it to only show the screen shot and if it is not changing then we do not show it until the new screenshot has a change relative to previous one . do i got it wright or correct me if i am wrong . and what to show if the screenshot does not changes .

[julienw]

what exactly is the task is it to only show the screen shot and if it is not changing then we do not show it until the new screenshot has a change relative to previous one . do i got it wright or correct me if i am wrong . and what to show if the screenshot does not changes .

Yes, this is exactly this!

If the screenshot does not change, I believe this could just be "blank", nothing drawn. We can decide later if we want to change the background to something grey instead of white, but that's easy to do later.

[Bucky25]

@julienw how are we going to compare screenshots .

[Bucky25]

@julienw hey can you help me with comparing screenshots

[julienw]

Hey @Bucky25, in this issue we don't want to compare? So I don't understand your question... Can you please explain this more?