Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layout 2020: Implement `clip: rect` #27388

Merged
merged 8 commits into from Jul 28, 2020
Merged

Layout 2020: Implement `clip: rect` #27388

merged 8 commits into from Jul 28, 2020

Conversation

@Manishearth
Copy link
Member

Manishearth commented Jul 24, 2020

This implements clip: rect

Unfortunately, none of the tests pass yet, they are all broken due to #27387

Additionally, currently clip does not seem to clip the element itself, only its children. I'm not quite sure what to do about that, I patterned this off of the code in the layout 2013 which handled clip immediately after scroll overflow.

@Manishearth Manishearth requested a review from SimonSapin Jul 24, 2020
@highfive
Copy link

highfive commented Jul 24, 2020

Heads up! This PR modifies the following files:

  • @jgraham: tests/wpt/include-layout-2020.ini
  • @emilio: components/style/values/computed/mod.rs, components/style/properties/longhands/effects.mako.rs, components/layout/display_list/builder.rs
@Manishearth Manishearth marked this pull request as draft Jul 24, 2020
@Manishearth
Copy link
Member Author

Manishearth commented Jul 24, 2020

@highfive highfive assigned SimonSapin and unassigned ferjm Jul 24, 2020
@Manishearth Manishearth changed the title Implement `clip: rect` Layout 2020: Implement `clip: rect` Jul 24, 2020
@Manishearth
Copy link
Member Author

Manishearth commented Jul 24, 2020

The last commit fixes the problem, but I'm not sure if it's the right fix.

@Manishearth Manishearth force-pushed the Manishearth:clip-2020 branch from 8872480 to 3a450f0 Jul 24, 2020
Copy link
Member

SimonSapin left a comment

As discussed on Matrix https://drafts.csswg.org/css2/#clipping says

Content that has been clipped does not cause overflow.

So this PR should also change scrollable_overflow computation for to take the intersection of the clip rect with what would otherwise be the scrollable overflow.

From::from(self.left.auto_is(|| Length::new(0.))),
From::from(self.top.auto_is(|| Length::new(0.))),
Comment on lines +834 to +835

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Jul 27, 2020

Member

Micro-nit: self.top.auto_is(Length::zero).into()

let clip = self.style.get_effects().clip;
if let ClipRectOrAuto::Rect(r) = clip {
Comment on lines 718 to 719

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Jul 27, 2020

Member

https://drafts.csswg.org/css2/#clipping

The clip property applies only to absolutely positioned elements.

So this code should also check for position: absolute or position: fixed.

@SimonSapin
Copy link
Member

SimonSapin commented Jul 27, 2020

Let’s try this on top of #27399

@bors-servo try=wpt-2020

bors-servo added a commit that referenced this pull request Jul 27, 2020
Layout 2020: Implement `clip: rect`

This implements `clip: rect`

Unfortunately, none of the tests pass yet, they are all broken due to #27387

Additionally, currently `clip` does not seem to clip the element itself, only its children. I'm not quite sure what to do about that, I patterned this off of the code in the layout 2013 which handled clip immediately after scroll overflow.
@bors-servo
Copy link
Contributor

bors-servo commented Jul 27, 2020

Trying commit 3a450f0 with merge d332319...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

Sweet, a whole pile of PASS expected FAIL. Though i bet many of these would have been PASS expected FAIL before this PR, I plan to rebase and do a two-step test update.

@Manishearth Manishearth force-pushed the Manishearth:clip-2020 branch from 3a450f0 to 0bc10b8 Jul 28, 2020
@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

PR updated with tests

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

So the clip-not-absolute-positioned tests are failing, because my impl seems to apply universally rather than to positioned elements

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

Fixed

@CYBAI
Copy link
Collaborator

CYBAI commented Jul 28, 2020

More PASS tests 👀

48 unexpected results that are NOT known-intermittents:
  â–¶ Unexpected subtest result in /css/css-transitions/properties-value-auto-001.html:
  â”” PASS [expected FAIL] clip auto(to) / events

  â–¶ Unexpected subtest result in /css/css-transitions/properties-value-auto-001.html:
  â”” PASS [expected FAIL] clip auto(from) / events


  â–¶ Unexpected subtest result in /css/cssom/serialize-values.html:
  â”” PASS [expected FAIL] clip: rect(1em, auto, 0.5px, 2000em)

  â–¶ Unexpected subtest result in /css/cssom/serialize-values.html:
  â”” PASS [expected FAIL] clip: auto

  â–¶ Unexpected subtest result in /css/cssom/serialize-values.html:
  â”” PASS [expected FAIL] clip: inherit


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-032.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-052.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-043.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-078.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-102.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-089.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-006.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-092.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-028.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-041.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-055.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-068.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-008.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-004.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-053.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-066.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-005.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-077.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-097.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-030.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-091.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-099.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-076.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-019.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-029.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-020.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-016.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-067.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-042.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-007.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-018.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-098.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-031.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-040.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-080.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-088.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-056.xht


  â–¶ PASS [expected FAIL] /_mozilla/css/clip_a.html


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-017.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-044.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-079.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-054.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-065.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-090.xht


  â–¶ PASS [expected FAIL] /css/CSS2/visufx/clip-064.xht


  â–¶ PASS [expected FAIL] /_mozilla/css/inline_absolute_hypothetical_clip_a.html

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

@bors-servo r=SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

📌 Commit 354c664 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

Testing commit 354c664 with merge 53593a4...

bors-servo added a commit that referenced this pull request Jul 28, 2020
Layout 2020: Implement `clip: rect`

This implements `clip: rect`

Unfortunately, none of the tests pass yet, they are all broken due to #27387

Additionally, currently `clip` does not seem to clip the element itself, only its children. I'm not quite sure what to do about that, I patterned this off of the code in the layout 2013 which handled clip immediately after scroll overflow.
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

@bors-servo retry

  ▶ TIMEOUT [expected ERROR] /css/cssom/CSSStyleSheet-constructable-disallow-import.tentative.html
  │ 
  └ _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
bors-servo added a commit that referenced this pull request Jul 28, 2020
Layout 2020: Implement `clip: rect`

This implements `clip: rect`

Unfortunately, none of the tests pass yet, they are all broken due to #27387

Additionally, currently `clip` does not seem to clip the element itself, only its children. I'm not quite sure what to do about that, I patterned this off of the code in the layout 2013 which handled clip immediately after scroll overflow.
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

Testing commit 354c664 with merge 4387342...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jul 28, 2020

@bors-servo retry

  │ ALSA lib pcm.c:2564:(snd_pcm_open_noupdate) Unknown PCM default
  │ AL lib: (EE) ALCplaybackAlsa_open: Could not open playback device 'default': No such file or directory
  │ 0:00:00.928360543  7914 0x7f1420013d00 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink0-actual-sink-openal> error: Could not open device.
  │ 0:00:00.928375573  7914 0x7f1420013d00 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink0-actual-sink-openal> error: ALC error: Out of Memory
  │ 0:00:00.929041095  7914 0x7f1420013d00 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink0-actual-sink-oss> error: Could not open audio device for playback.
  │ 0:00:00.929052884  7914 0x7f1420013d00 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink0-actual-sink-oss> error: system error: No such file or directory
  │ 0:00:00.939149462  7914 0x7f1420006d40 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc0:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ Cannot connect to server socket err = No such file or directory
  │ Cannot connect to server request channel
  │ jack server is not running or cannot be started
  │ JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
  │ JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
  │ 0:00:00.961377010  7914 0x7f1418002a70 WARN              jackclient gstjackaudioclient.c:381:gst_jack_audio_get_connection: could not create connection
  │ 0:00:00.961754417  7914 0x7f1418002a70 WARN                jacksink gstjackaudiosink.c:355:gst_jack_ring_buffer_open_device:<autoaudiosink1-actual-sink-jackaudio> error: Jack server not found
  │ 0:00:00.961826626  7914 0x7f1418002a70 WARN                jacksink gstjackaudiosink.c:355:gst_jack_ring_buffer_open_device:<autoaudiosink1-actual-sink-jackaudio> error: Cannot connect to the Jack server (status 17)
  │ 0:00:00.962657478  7914 0x7f1418002a70 WARN                 default oss4-property-probe.c:302:gst_oss4_property_probe_get_values:<autoaudiosink1-actual-sink-oss4> Can't open file descriptor to probe available devices: No such file or directory
  │ 0:00:00.962690030  7914 0x7f1418002a70 WARN                oss4sink oss4-sink.c:513:gst_oss4_sink_open:<autoaudiosink1-actual-sink-oss4> error: Could not open audio device for playback.
  │ 0:00:00.962697959  7914 0x7f1418002a70 WARN                oss4sink oss4-sink.c:513:gst_oss4_sink_open:<autoaudiosink1-actual-sink-oss4> error: system error: No such file or directory
  │ ALSA lib confmisc.c:767:(parse_card) cannot find card '0'
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_card_driver returned error: No such file or directory
  │ ALSA lib confmisc.c:392:(snd_func_concat) error evaluating strings
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory
  │ ALSA lib confmisc.c:1246:(snd_func_refer) error evaluating name
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory
  │ ALSA lib conf.c:5047:(snd_config_expand) Evaluate error: No such file or directory
  │ ALSA lib pcm.c:2564:(snd_pcm_open_noupdate) Unknown PCM default
  │ AL lib: (EE) ALCplaybackAlsa_open: Could not open playback device 'default': No such file or directory
  │ 0:00:00.963222527  7914 0x7f1418002a70 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink1-actual-sink-openal> error: Could not open device.
  │ 0:00:00.963232102  7914 0x7f1418002a70 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink1-actual-sink-openal> error: ALC error: Out of Memory
  │ 0:00:00.963345964  7914 0x7f1418002a70 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink1-actual-sink-oss> error: Could not open audio device for playback.
  │ 0:00:00.963354746  7914 0x7f1418002a70 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink1-actual-sink-oss> error: system error: No such file or directory
  │ 0:00:00.963894965  7914 0x7f14180074f0 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc1:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.985385018  7914 0x7f1418007450 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc2:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.988049760  7914 0x7f1418007c00 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc3:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.989589679  7914 0x7f14180074a0 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc4:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.989827861  7914 0x7f1418007680 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc5:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.990594814  7914 0x7f141802fa80 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc6:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.992381306  7914 0x7f14180300a0 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc7:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  └ 0:00:00.993424085  7914 0x7f1418007cf0 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc8:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

Testing commit 354c664 with merge b41f5f9...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

☀️ Test successful - status-taskcluster
Approved by: SimonSapin
Pushing b41f5f9 to master...

@bors-servo bors-servo merged commit b41f5f9 into servo:master Jul 28, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.