rebenitez1802 commented on code in PR #44034:
URL: https://github.com/apache/superset/pull/44034#discussion_r3966410145
##########
superset/utils/screenshot_utils.py:
##########
@@ -130,26 +137,91 @@ class ScreenshotCaptureTimeoutError(RuntimeError):
"""Raised when Chromium repeatedly times out while capturing a tile."""
-def is_screenshot_nearly_uniform(screenshot: bytes) -> tuple[bool, float]:
- """Return whether one color occupies nearly all sampled screenshot
pixels."""
+class ScreenshotBlankCaptureError(RuntimeError):
+ """Raised when Chromium repeatedly returns a perceptually blank capture."""
+
+
+@dataclass(frozen=True)
+class ScreenshotBlanknessMetrics:
+ """Metrics used to decide whether a screenshot is perceptually blank."""
+
+ is_blank: bool
+ dominant_pixel_ratio: float
+ near_white_pixel_ratio: float
+ mean_luminance: float
+ luminance_stddev: float
+ entropy: float
+
+
+def get_screenshot_blankness_metrics(screenshot: bytes) ->
ScreenshotBlanknessMetrics:
+ """Measure exact-color and perceptual blankness on a sampled screenshot."""
try:
with Image.open(io.BytesIO(screenshot)) as image:
- sample = image.convert("RGB")
- sample.thumbnail((256, 256))
- colors = sample.getcolors(maxcolors=256)
- if not colors:
- return False, 0.0
- dominant_pixels = max(count for count, _color in colors)
- dominant_ratio = dominant_pixels / (sample.width * sample.height)
- return (
- dominant_ratio >= TILED_SCREENSHOT_BLANK_DOMINANT_PIXEL_RATIO,
- dominant_ratio,
+ sample_metrics: list[ScreenshotBlanknessMetrics] = []
+ for sample_size in SCREENSHOT_BLANK_SAMPLE_SIZES:
+ sample = image.convert("RGB")
+ sample.thumbnail((sample_size, sample_size))
+ pixel_count = sample.width * sample.height
+ colors = sample.getcolors(maxcolors=pixel_count) or []
+ dominant_pixels = max(
+ (count for count, _color in colors),
+ default=0,
+ )
+ dominant_ratio = dominant_pixels / pixel_count
+
+ grayscale = sample.convert("L")
+ histogram = grayscale.histogram()
+ near_white_ratio = (
+ sum(histogram[SCREENSHOT_BLANK_MIN_LUMINANCE:]) /
pixel_count
+ )
+ statistics = ImageStat.Stat(grayscale)
+ mean_luminance = float(statistics.mean[0])
+ luminance_stddev = float(statistics.stddev[0])
+ entropy = -sum(
+ (count / pixel_count) * math.log2(count / pixel_count)
+ for count in histogram
+ if count
+ )
+ low_information = (
+ luminance_stddev <= SCREENSHOT_BLANK_MAX_LUMINANCE_STDDEV
+ and entropy <= SCREENSHOT_BLANK_MAX_ENTROPY
+ )
+ perceptually_blank = low_information and (
Review Comment:
🟡 **Medium (non-blocking — your call):** this fail-closed rejection can
deterministically fail a narrow class of *legitimate* light/sparse reports,
with no operator opt-out. `perceptually_blank = (stddev≤8 and entropy≤1.5) and
(mean≥250 or dominant≥0.995)` — common charts survive (line/scatter with axes,
Big Number, bordered tables, all verified), but genuinely sparse low-chrome
near-white content (a sparse scatter with minimal axes, a borderless text
table, a very-light pastel area chart) reads blank at *both* sample scales → 3
retries over identical pixels → `ScreenshotBlankCaptureError` →
`ReportScheduleScreenshotFailedError` on every run, and there's no config flag
to turn perceptual rejection off. Worth considering a config/feature flag as a
safety valve, and/or gating on a resolution-independent ink fraction rather
than the DPI-sensitive global stddev/entropy. Not blocking the approval.
##########
tests/unit_tests/utils/test_screenshot_utils.py:
##########
@@ -68,6 +79,66 @@ def test_chart_like_png_is_not_blank(self):
assert is_blank is False
assert dominant_ratio < 0.995
+ def test_two_tone_near_white_png_is_blank(self):
+ is_blank, dominant_ratio =
is_screenshot_nearly_uniform(_two_tone_blank())
+
+ assert is_blank is True
+ assert dominant_ratio == 0.85
+
+ def test_two_tone_gray_below_near_white_cutoff_is_blank(self):
+ image = Image.new("RGB", (100, 100), "white")
+ for y in range(85, 100):
+ for x in range(100):
+ image.putpixel((x, y), (239, 239, 239))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is True
+ assert dominant_ratio == 0.85
+
+ def test_sparse_readable_text_is_not_blank(self):
+ image = Image.new("RGB", (800, 1000), "white")
+ label = Image.new("RGB", (60, 14), "white")
+ ImageDraw.Draw(label).text(
+ (0, 0), "No data", fill="black", font=ImageFont.load_default()
+ )
+ label = label.resize((240, 56), Image.Resampling.NEAREST)
+ image.paste(label, (20, 20))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, _dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is False
+
+ def test_tall_sparse_report_with_content_is_not_blank(self):
Review Comment:
🟡 **Medium (non-blocking):** this "sparse" test passes only because its
strokes are thick and black — swap them to thin/light and it flips to
`is_blank=True`. Together with `_create_chart_like_tile` (a dense ~60%-colored
block), the suite has no fixture asserting that a realistic *light thin-stroke*
line/scatter or a *sparse table* at report resolution is **not** blank — so the
false-positive class flagged on `screenshot_utils.py` line 190 would ship
green. Worth adding light thin-stroke line/scatter + sparse-table fixtures at
~3000×1200 (and `pixel_density=2`) that assert `is_blank` is False, and using
them to calibrate the thresholds. Your call.
##########
superset/utils/screenshot_utils.py:
##########
@@ -1224,6 +1342,27 @@ def _raise_if_budget_exhausted() -> None:
log_context=log_context,
)
+ if report_execution_context and contentful_tiles_captured:
Review Comment:
🟡 **Medium (non-blocking):** the combined-image check gates only on
`contentful_tiles_captured`, not on what fraction of the page is actually
content. A tall / sparse-ink dashboard (a few real charts among empty / spacer
/ markdown tiles, or many low-chrome charts) can wash out at the 1024px
downsample so the *combined* image scores blank and raises here — even though
every tile individually passed its per-tile check. This is new behavior (base
`master` just returned the combined image). Consider basing the combined
decision on aggregate non-near-white ink (or on the per-tile results already
computed), or making the combined check advisory/log-only once tiles have
individually passed. Your call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
