Skip to content

feat: redraw the result image in the modern design - #833

Open
BKPepe wants to merge 2 commits into
librespeed:masterfrom
BKPepe:feat/modern-result-image
Open

feat: redraw the result image in the modern design#833
BKPepe wants to merge 2 commits into
librespeed:masterfrom
BKPepe:feat/modern-result-image

Conversation

@BKPepe

@BKPepe BKPepe commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The shared result image still draws black on white in the classic style, while
index-modern.html and frontend/styling/ describe a dark interface. Sharing a
result posts an image that no longer looks like the site it came from.

before after
before after

Colours are taken from frontend/styling/colors.css rather than invented, so
the two stay in step.

Three things the image already carried but never showed are now drawn, all from
columns the query selects today: the client, the address family, and a
timestamp split into a date and a time.

Rendered at 3x and scaled down, because GD antialiases nothing it fills and
circles and rounded corners would otherwise come out stepped. No new
dependency, and the same two fonts already in results/.

Edge cases, rendered: a browser user agent, which is shortened to fit, a long
provider name, and redact_ip_addresses on, where the family is dropped rather
than reporting the stored placeholder as IPv4.

edge cases

The panels carry a gradient rather than a sparkline on purpose: a curve inside a
result reads as measured data, and no per-second samples are kept for a shared
result, so it would be invented.

Stacked on #832. Happy to split the client and address family out onto the
classic card instead if the redesign is not wanted.

Copilot AI lite review requested due to automatic review settings August 10, 2026 00:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Redesign shared result PNG to match modern dark UI

✨ Enhancement 🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Redraw the shared result PNG using the modern dark palette from frontend styling.
• Add client, address family, and readable date/time to the image footer/header.
• Normalize timestamps by stripping fractional seconds for cross-database consistency.
Diagram

graph TD
  A["Share image request"] --> B["results/index.php"] --> C["telemetry_db.php"] --> D[("Telemetry DB")] --> E["GD renderer"] --> F["PNG response"]
  B --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Render an HTML template and screenshot it (headless browser)
  • ➕ Much easier to keep styling in lockstep with the frontend CSS
  • ➕ Native antialiasing for text and shapes; fewer manual geometry helpers
  • ➖ Adds heavy runtime dependencies and operational complexity
  • ➖ Harder to run in constrained/shared hosting environments
2. Generate SVG instead of rasterizing with GD
  • ➕ Vector output scales cleanly; avoids the 3× oversampling workaround
  • ➕ Simpler rounded corners/gradients via SVG primitives
  • ➖ Requires an SVG rendering strategy for PNG-only consumers (or format change)
  • ➖ More work to ensure font availability/metrics match across environments

Recommendation: Given the stated constraint of “no new dependency” and broad deployability, the chosen GD-based redraw is appropriate. If this grows further, consider extracting the drawing helpers into a dedicated renderer module/file to keep the endpoint maintainable.

Files changed (1) +467 / -137

Enhancement (1) +467 / -137
index.phpModernize shared result image rendering and timestamp handling +467/-137

Modernize shared result image rendering and timestamp handling

• Replaces the classic white result card with a modern dark design using the same palette as the frontend. Adds timestamp normalization (strip fractional seconds), splits timestamp into date/time, derives and conditionally displays IP address family, and renders client/user-agent text with truncation. Introduces GD drawing utilities (rounded panels, badges, gradients) and renders at 3× resolution before downsampling for smoother edges.

results/index.php

formatSpeedtestDataForImage() assigned the timestamp to itself, so the value
reached the image exactly as the database returned it. Every other field on
that list is passed through format(); this one reads like a placeholder that
was never filled in, and static analysis flags it as a self-assignment.

How much precision the column carries is decided by the backend, and the three
schemas shipped here disagree: MySQL's `timestamp` keeps none, PostgreSQL's
`timestamp without time zone DEFAULT now()` keeps microseconds and MSSQL's
`datetime` keeps milliseconds. A PostgreSQL deployment therefore drew
"2026-08-10 02:04:13.957789" where a MySQL one drew "2026-08-10 02:04:13".
@BKPepe
BKPepe force-pushed the feat/modern-result-image branch from 46b1144 to 80f50ff Compare August 10, 2026 00:45
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Polygon call incompatible 🐞 Bug ☼ Reliability
Description
directionBadge() calls imagefilledpolygon() with the 3-argument form, which can break deployments on
older PHP versions that this repo’s docs still claim to support. On such systems, generating
/results/?id=… can error out before producing an image.
Code

results/index.php[R316-320]

+    imagefilledpolygon($im, [
+        (int) $cx, (int) ($tipY + $dir * $head * 0.5),
+        (int) ($cx - $head), (int) ($tipY - $dir * $head * 0.55),
+        (int) ($cx + $head), (int) ($tipY - $dir * $head * 0.55),
+    ], $color);
Evidence
The PR introduces a 3-argument imagefilledpolygon call in the new drawing code, while the
repository documentation still claims PHP 5.4+ compatibility, which implies older GD function
signatures may be in use in supported deployments.

results/index.php[300-321]
README.md[32-37]
doc.md[45-58]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`directionBadge()` uses `imagefilledpolygon($im, $points, $color)`. Some PHP/GD versions require the legacy signature `imagefilledpolygon($im, $points, $num_points, $color)`. Since project docs still state PHP 5.4+ support, this can make result image generation fail on those deployments.

### Issue Context
- The code path is executed for every rendered image (download/upload badges).
- Docs still describe supporting older PHP versions.

### Fix Focus Areas
- results/index.php[300-321]

### Suggested fix
Use a version-safe call, e.g.:
- Build `$points = [...]` once
- If `PHP_VERSION_ID >= 80000` call the 3-arg variant; else call the 4-arg variant with `$numPoints = (int)(count($points)/2)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Costly byte truncation 🐞 Bug ➹ Performance
Description
fitText() shrinks strings by repeatedly removing a byte with substr() and re-measuring via
imageftbbox(), which is inefficient for long inputs and can split multibyte UTF-8 characters. This
matters because the rendered client string comes directly from the stored HTTP_USER_AGENT and can be
arbitrarily long.
Code

results/index.php[R373-376]

+    while ('' !== $text && textWidth($text.'…', $font, $size) > $maxWidth) {
+        $text = substr($text, 0, -1);
+    }
+
Evidence
The new fitText() function performs per-character removal with substr() and repeatedly calls
textWidth() (which calls imageftbbox()). The string it truncates includes the stored ua, which
comes from HTTP_USER_AGENT and is not bounded in telemetry ingestion.

results/index.php[368-378]
results/index.php[522-533]
results/telemetry.php[7-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`fitText()` truncates with `substr($text, 0, -1)` in a loop and calls `imageftbbox()` each iteration. This is inefficient for long strings and byte-based truncation can corrupt UTF-8 sequences.

### Issue Context
- `drawImage()` applies `fitText()` to the stored user-agent (`$data['ua']`).
- Telemetry stores `ua` directly from `$_SERVER['HTTP_USER_AGENT']`.

### Fix Focus Areas
- results/index.php[360-378]
- results/index.php[522-533]
- results/telemetry.php[7-12]

### Suggested fix
- Impose a hard maximum length before measuring (e.g., cap UA to N characters).
- Truncate by Unicode characters (e.g., use `preg_split('//u', ...)` and pop chars) rather than `substr` bytes.
- Reduce measurement calls (e.g., binary search for the longest fitting prefix instead of per-character iteration).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread results/index.php Outdated
Comment on lines +316 to +320
imagefilledpolygon($im, [
(int) $cx, (int) ($tipY + $dir * $head * 0.5),
(int) ($cx - $head), (int) ($tipY - $dir * $head * 0.55),
(int) ($cx + $head), (int) ($tipY - $dir * $head * 0.55),
], $color);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Polygon call incompatible 🐞 Bug ☼ Reliability

directionBadge() calls imagefilledpolygon() with the 3-argument form, which can break deployments on
older PHP versions that this repo’s docs still claim to support. On such systems, generating
/results/?id=… can error out before producing an image.
Agent Prompt
### Issue description
`directionBadge()` uses `imagefilledpolygon($im, $points, $color)`. Some PHP/GD versions require the legacy signature `imagefilledpolygon($im, $points, $num_points, $color)`. Since project docs still state PHP 5.4+ support, this can make result image generation fail on those deployments.

### Issue Context
- The code path is executed for every rendered image (download/upload badges).
- Docs still describe supporting older PHP versions.

### Fix Focus Areas
- results/index.php[300-321]

### Suggested fix
Use a version-safe call, e.g.:
- Build `$points = [...]` once
- If `PHP_VERSION_ID >= 80000` call the 3-arg variant; else call the 4-arg variant with `$numPoints = (int)(count($points)/2)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread results/index.php Outdated
Comment on lines +373 to +376
while ('' !== $text && textWidth($text.'…', $font, $size) > $maxWidth) {
$text = substr($text, 0, -1);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Costly byte truncation 🐞 Bug ➹ Performance

fitText() shrinks strings by repeatedly removing a byte with substr() and re-measuring via
imageftbbox(), which is inefficient for long inputs and can split multibyte UTF-8 characters. This
matters because the rendered client string comes directly from the stored HTTP_USER_AGENT and can be
arbitrarily long.
Agent Prompt
### Issue description
`fitText()` truncates with `substr($text, 0, -1)` in a loop and calls `imageftbbox()` each iteration. This is inefficient for long strings and byte-based truncation can corrupt UTF-8 sequences.

### Issue Context
- `drawImage()` applies `fitText()` to the stored user-agent (`$data['ua']`).
- Telemetry stores `ua` directly from `$_SERVER['HTTP_USER_AGENT']`.

### Fix Focus Areas
- results/index.php[360-378]
- results/index.php[522-533]
- results/telemetry.php[7-12]

### Suggested fix
- Impose a hard maximum length before measuring (e.g., cap UA to N characters).
- Truncate by Unicode characters (e.g., use `preg_split('//u', ...)` and pop chars) rather than `substr` bytes.
- Reduce measurement calls (e.g., binary search for the longest fitting prefix instead of per-character iteration).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Replace the outdated classic result image with the modern dark styling
used by the frontend.

Reuse the existing color palette and display the client, address family,
and timestamp already available in the result data.
@BKPepe
BKPepe force-pushed the feat/modern-result-image branch from 80f50ff to c9606ae Compare August 10, 2026 01:02
@sstidl

sstidl commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

I like the design.
As long as we support old and new design we should have both designs available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants