Skip to content

line halo#870

Open
Fil wants to merge 2 commits intomainfrom
fil/line-halo
Open

line halo#870
Fil wants to merge 2 commits intomainfrom
fil/line-halo

Conversation

@Fil
Copy link
Contributor

@Fil Fil commented May 13, 2022

Add a line halo. See documentation and test examples.

closes #860

a follow-up could be to add the halo option to other "non-fillable" marks: Plot.link, Plot.arrow, Plot.tick, Plot.rule…

line chart with faint halo


EDIT: this PR was completely rewritten, obsolete description

An unexpected difficulty was to make it compatible with group aesthetics. I had to pass a flag (in the shape of a counter) to secondary groups. For now I just "walk back to the first element", and don't use the counter per se, because we might want to switch to a real flag that would be absent from the first group—which might be more satisfying?

@Fil Fil requested review from awiederkehr and mbostock May 13, 2022 17:10
@Fil
Copy link
Contributor Author

Fil commented May 16, 2022

A problem currently is that the halo option can be an object containing anything. I don't know how to check that the values are numbers or strings as we usually do for normal options.

@Fil Fil added the question Further information is needed label May 16, 2022
@Fil Fil marked this pull request as ready for review May 16, 2022 09:34
@Fil Fil added the enhancement New feature or request label May 16, 2022
@Fil Fil marked this pull request as draft May 28, 2022 16:01
@Fil
Copy link
Contributor Author

Fil commented May 28, 2022

scratch that, a much better solution will be to use a svg filter (#409). I have a prototype here https://observablehq.com/@fil/line-halo-in-pure-svg

@Fil
Copy link
Contributor Author

Fil commented Jul 11, 2022

new version with the SVG filter strategy

@Fil Fil marked this pull request as ready for review July 11, 2022 13:06
@Fil Fil force-pushed the fil/line-halo branch 2 times, most recently from e2eb2da to 4f7ecbd Compare October 18, 2022 00:20
@Fil
Copy link
Contributor Author

Fil commented Oct 18, 2022

manually rebased

@Fil Fil mentioned this pull request Feb 6, 2023
Comment on lines +9 to +15
<feMorphology in="SourceAlpha" result="DILATED" operator="dilate" radius="${radius}"></feMorphology>
<feFlood flood-color="${color}" result="BG"></feFlood>
<feComposite in="BG" in2="DILATED" operator="in" result="OUTLINE"></feComposite>
<feMerge>
<feMergeNode in="OUTLINE" />
<feMergeNode in="SourceGraphic" />
</feMerge>`);
Copy link
Member

Choose a reason for hiding this comment

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

You only need the result+in for dilated; otherwise in defaults to the previous filter primitive.

Suggested change
<feMorphology in="SourceAlpha" result="DILATED" operator="dilate" radius="${radius}"></feMorphology>
<feFlood flood-color="${color}" result="BG"></feFlood>
<feComposite in="BG" in2="DILATED" operator="in" result="OUTLINE"></feComposite>
<feMerge>
<feMergeNode in="OUTLINE" />
<feMergeNode in="SourceGraphic" />
</feMerge>`);
<feMorphology in="SourceAlpha" result="dilated" operator="dilate" radius="${radius}"></feMorphology>
<feFlood flood-color="${color}"></feFlood>
<feComposite in2="dilated" operator="in"></feComposite>
<feMerge>
<feMergeNode></feMergeNode>
<feMergeNode in="SourceGraphic"></feMergeNode>
</feMerge>`);

);

if (this.halo) {
// With variable aesthetics, we need to regroup segments by line
Copy link
Member

@mbostock mbostock Apr 1, 2023

Choose a reason for hiding this comment

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

This is hard to follow. I think it would be clearer to initialize the DOM the desired way from the beginning, rather than creating it once (above using groupIndex) and then moving things around afterwards to apply the halo. (Especially since this already requires changing groupIndex to populate a segment property on the data, whose purpose isn’t clear until you go look at the code here.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rewrote the whole thing, PTAL

@mbostock
Copy link
Member

mbostock commented Apr 1, 2023

This seems like a nice feature. The implementation needs some tidying.

@Fil Fil marked this pull request as draft April 3, 2023 14:42
@Fil
Copy link
Contributor Author

Fil commented Jul 31, 2023

A similar situation arises with the line opacity when the line is broken into segments for varying aesthetics: https://talk.observablehq.com/t/opacity-artifacts-in-line-segments/8142/2

@Fil Fil marked this pull request as ready for review March 12, 2026 15:47
@Fil Fil requested review from allisonhorst and removed request for awiederkehr March 12, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Halo for lines?

2 participants