Skip to content

Commit

Permalink
Text rendering: fixes and improvements
Browse files Browse the repository at this point in the history
Introduces a new option to disable the pixel comparison text visibility
detecting code. There are some situations in which it helps and some in
which it hinders. So for now it is disable by default and can be
enabled through the config file or my pressing F6 in the TTY client.

Also includes a couple of fixes to the HTTP server's text rendering.
  • Loading branch information
tombh committed Jun 18, 2019
1 parent d93c084 commit f8b0e55
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 8 deletions.
5 changes: 5 additions & 0 deletions interfacer/src/browsh/config_sample.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ mobile_user_agent = "Mozilla/5.0 (Android 7.0; Mobile; rv:54.0) Gecko/58.0 Firef
[browsh] # Browsh internals
websocket-port = 3334
# Possibly better handling of overlapping text in web pages. If a page seems to have
# text that shouldn't be visible, if it should be behind another element for example,
# then this experimental feature should help. It can also be toggled in-browser with F6.
use_experimental_text_visibility = false
[firefox]
# The path to your Firefox binary
path = "firefox"
Expand Down
4 changes: 2 additions & 2 deletions webext/src/background/dimensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ export default class extends utils.mixins(CommonMixin) {
this.char.width != incoming.width ||
this.char.height != incoming.height
) {
this.char = _.clone(incoming);
this.log(
`Requesting browser resize for new char dimensions: ` +
`${this.char.width}x${this.char.height}`
`${incoming.width}x${incoming.height} (old: ${this.char.width}x${this.char.height})`
);
this.char = _.clone(incoming);
this.resizeBrowserWindow();
}
}
Expand Down
9 changes: 9 additions & 0 deletions webext/src/dom/commands_mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,19 @@ export default MixinBase =>
}

_handleSpecialKeys(input) {
let state, message;
switch (input.key) {
case 18: // CTRL+r
window.location.reload();
break;
case 284: // F6
state = this.config.browsh.use_experimental_text_visibility;
state = !state;
this.config.browsh.use_experimental_text_visibility = state;
message = state ? 'on' : 'off'
this.sendMessage(`/status,info,Experimental text visibility: ${message}`);
this.sendSmallTextFrame();
break;
}
}

Expand Down
19 changes: 14 additions & 5 deletions webext/src/dom/text_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) {
this.dimensions = dimensions;
this.graphics_builder = graphics_builder;
this.config = config;
this.tty_grid = new TTYGrid(dimensions, graphics_builder);
this.tty_grid = new TTYGrid(dimensions, graphics_builder, config);
this._parse_started_elements = [];
// A `range` is the DOM's representation of elements and nodes as they are rendered in
// the DOM. Think of the 'range' that is created when you select/highlight text for
Expand All @@ -35,8 +35,8 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) {

buildFormattedText(callback) {
this._updateState();
this._getTextNodes();
this.graphics_builder.getOnOffScreenshots(() => {
this._getTextNodes();
this._positionTextNodes();
callback();
});
Expand Down Expand Up @@ -162,7 +162,9 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) {
// * Yet another thing, the style change doesn't actually get picked up until the
// next frame. Thus why the loop is independent of the `positionTextNodes()` loop.
_fixJustifiedText() {
this._node.parentElement.style.textAlign = "left";
if (this._node.parentElement) {
this._node.parentElement.style.textAlign = "left";
}
}

// The need for this wasn't immediately obvious to me. The fact is that the DOM stores
Expand Down Expand Up @@ -246,8 +248,15 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) {
// Although do note that, unlike selection ranges, sub-selections can appear seemingly
// inside other selections for things like italics or anchor tags.
_getNodeDOMBoxes() {
this._range.selectNode(this._node);
return this._range.getClientRects();
let rects = []
// TODO: selectNode() hangs if it can't find a node in the DOM
// Node.isConnected() might be faster
// It's possible that the node has dissapeared since nodes were collected.
if (document.body.contains(this._node)){
this._range.selectNode(this._node);
rects = this._range.getClientRects();
}
return rects;
}

// A single box is always a valid rectangle. Therefore a single box will, for example,
Expand Down
6 changes: 5 additions & 1 deletion webext/src/dom/tty_grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import utils from "utils";

// The TTY grid
export default class {
constructor(dimensions, graphics_builder) {
constructor(dimensions, graphics_builder, config) {
this.dimensions = dimensions;
this.graphics_builder = graphics_builder;
this.config = config;
this._setMiddleOfEm();
}

Expand Down Expand Up @@ -98,6 +99,9 @@ export default class {
// case we can work with `z-index` so that characters justifiably overwrite each other in
// the TTY grid.
_isCharObscured(colours) {
if (!this.config.browsh.use_experimental_text_visibility) {
return false
}
return (
colours[0][0] === colours[1][0] &&
colours[0][1] === colours[1][1] &&
Expand Down

0 comments on commit f8b0e55

Please sign in to comment.