Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Sep 12, 2022

This change makes Flutter web render tabs as a single space. Previously, tabs rendered differently depending on your platform:

  • Windows, Web - a tofu is rendered
  • iOS, Android, macOS, Linux - a single space is rendered
This change was tested on this repro...
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      title: 'Flutter Demo',
      home: Text('\tHello\tworld\t'),
    );
  }
}

Here was the tab tofu fix for Windows: #34389

Part of flutter/flutter#79153.

Background

Most fonts don't have a glyph for \t, so the "unknown" glyph should be selected and a tofu should be drawn. However, some platforms will override this using their own platform-specific logic and select the glyph for a single space instead. For more information, see: https://bugs.chromium.org/p/skia/issues/detail?id=12334

Skia added a feature to substitute tabs with a single space so that platforms could behave consistently. This feature was added to CanvasKit in version 0.37.0 (skia#574296). This change enables this new feature on Flutter web, thereby making tabs no longer render as a tofu.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Sep 12, 2022
}
}
return true;
}
Copy link
Member Author

@loic-sharma loic-sharma Sep 12, 2022

Choose a reason for hiding this comment

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

This is a copy of:

/// @returns true When the images are reasonably similar.
/// @todo Make the search actually fuzzy to a certain degree.
Future<bool> fuzzyCompareImages(Image golden, Image img) async {
if (golden.width != img.width || golden.height != img.height) {
return false;
}
int getPixel(ByteData data, int x, int y) => data.getUint32((x + y * golden.width) * 4);
final ByteData goldenData = (await golden.toByteData())!;
final ByteData imgData = (await img.toByteData())!;
for (int y = 0; y < golden.height; y++) {
for (int x = 0; x < golden.width; x++) {
if (getPixel(goldenData, x, y) != getPixel(imgData, x, y)) {
return false;
}
}
}
return true;
}

Please let me know if there's a way to deduplicate this by sharing code between lib/web_ui and testing/dart!

@loic-sharma loic-sharma marked this pull request as ready for review September 13, 2022 17:50
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

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

LGTM. Do you know if replaceTabCharacters will use the font's tab glyph if one is available? Or does it do the replacement no matter what?

@loic-sharma
Copy link
Member Author

@hterkelsen It'll do the replacement no matter what, which is consistent with how the other platforms work now.

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 14, 2022
@auto-submit auto-submit bot merged commit 46037aa into flutter:main Sep 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 14, 2022
@loic-sharma loic-sharma deleted the web_tab_tofu branch September 16, 2022 16:44
Oleh-Sv pushed a commit to Oleh-Sv/engine that referenced this pull request Sep 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants