Improve how intrinsic sizes work for videos (#31746)

* feat: patch for video layout sizes

added rebase from main 2024/10/05

Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Signed-off-by: eri <epazos@igalia.com>

* feat: take width and height parameters if provided

Signed-off-by: eri <epazos@igalia.com>

* chore: tidy the code and update test expectations

Signed-off-by: eri <epazos@igalia.com>

* feat: handle removing poster

Signed-off-by: eri <epazos@igalia.com>

* chore: update test expectations and remove debug code

Signed-off-by: eri <epazos@igalia.com>

* fix: issues after rebasing to main

Signed-off-by: eri <epazos@igalia.com>

* feat: pass src remove test and tidy

Signed-off-by: eri <epazos@igalia.com>

* chore: clippy fixes

Signed-off-by: eri <epazos@igalia.com>

* chore: update passing test expectations

Signed-off-by: eri <epazos@igalia.com>

* fix object-position-svg test

Signed-off-by: eri <epazos@igalia.com>

* fix unintentional override of video size and resize events

Signed-off-by: eri <epazos@igalia.com>

* change how resize events are sent to better match the spec

Signed-off-by: eri <epazos@igalia.com>

* simplify poster mutation handling

Co-authored-by: Oriol Brufau <obrufau@igalia.com>
Signed-off-by: eri <eri@inventati.org>

* improved handling of intrinsic sizes

- differentiate between natural size and css size
- presentational attributes
- fallback ratio for video element
- handle more cases where the src/poster are added/removed
- aspect ratio hints

Signed-off-by: eri <epazos@igalia.com>

* update test expectations

Signed-off-by: eri <epazos@igalia.com>

* fix cleaning current frame

Signed-off-by: eri <epazos@igalia.com>

* update test expectations

Signed-off-by: eri <epazos@igalia.com>

* Apply suggestions from code review

Co-authored-by: Oriol Brufau <obrufau@igalia.com>
Signed-off-by: eri <eri@inventati.org>

* More code review suggestions

Signed-off-by: eri <epazos@igalia.com>

* Prevent aspect-ratio:auto from pulling the ratio from the default object size

As resolved in https://github.com/w3c/csswg-drafts/issues/7524#issuecomment-1204462924

Signed-off-by: Oriol Brufau <obrufau@igalia.com>

---------

Signed-off-by: eri <epazos@igalia.com>
Signed-off-by: eri <eri@inventati.org>
Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
eri 2024-10-29 22:42:22 +00:00 committed by GitHub
parent 43d1601016
commit 01820e2a8a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 290 additions and 360 deletions

View file

@ -55,6 +55,8 @@ use style::selector_parser::{
use style::shared_lock::{Locked, SharedRwLock};
use style::stylesheets::layer_rule::LayerOrder;
use style::stylesheets::{CssRuleType, UrlExtraData};
use style::values::generics::position::PreferredRatio;
use style::values::generics::ratio::Ratio;
use style::values::generics::NonNegative;
use style::values::{computed, specified, AtomIdent, AtomString, CSSFloat};
use style::{dom_apis, thread_state, ArcSlice, CaseSensitivityExt};
@ -132,6 +134,7 @@ use crate::dom::htmltablesectionelement::{
};
use crate::dom::htmltemplateelement::HTMLTemplateElement;
use crate::dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaElementHelpers};
use crate::dom::htmlvideoelement::{HTMLVideoElement, LayoutHTMLVideoElementHelpers};
use crate::dom::mutationobserver::{Mutation, MutationObserver};
use crate::dom::namednodemap::NamedNodeMap;
use crate::dom::node::{
@ -849,6 +852,8 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
this.get_width()
} else if let Some(this) = self.downcast::<HTMLImageElement>() {
this.get_width()
} else if let Some(this) = self.downcast::<HTMLVideoElement>() {
this.get_width()
} else if let Some(this) = self.downcast::<HTMLTableElement>() {
this.get_width()
} else if let Some(this) = self.downcast::<HTMLTableCellElement>() {
@ -891,6 +896,8 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
this.get_height()
} else if let Some(this) = self.downcast::<HTMLImageElement>() {
this.get_height()
} else if let Some(this) = self.downcast::<HTMLVideoElement>() {
this.get_height()
} else if let Some(this) = self.downcast::<HTMLTableElement>() {
this.get_height()
} else if let Some(this) = self.downcast::<HTMLTableCellElement>() {
@ -927,6 +934,27 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
},
}
// Aspect ratio when providing both width and height.
// https://html.spec.whatwg.org/multipage/#attributes-for-embedded-content-and-images
if self.downcast::<HTMLImageElement>().is_some() ||
self.downcast::<HTMLVideoElement>().is_some()
{
if let LengthOrPercentageOrAuto::Length(width) = width {
if let LengthOrPercentageOrAuto::Length(height) = height {
let width_value = NonNegative(specified::Number::new(width.to_f32_px()));
let height_value = NonNegative(specified::Number::new(height.to_f32_px()));
let aspect_ratio = specified::position::AspectRatio {
auto: true,
ratio: PreferredRatio::Ratio(Ratio(width_value, height_value)),
};
hints.push(from_declaration(
shared_lock,
PropertyDeclaration::AspectRatio(aspect_ratio),
));
}
}
}
let cols = if let Some(this) = self.downcast::<HTMLTextAreaElement>() {
match this.get_cols() {
0 => None,

View file

@ -27,7 +27,7 @@ use net_traits::{
ResourceTimingType,
};
use pixels::Image;
use script_layout_interface::HTMLMediaData;
use script_layout_interface::MediaFrame;
use servo_config::pref;
use servo_media::player::audio::AudioRenderer;
use servo_media::player::video::{VideoFrame, VideoFrameRenderer};
@ -68,7 +68,7 @@ use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::num::Finite;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{Dom, DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::blob::Blob;
use crate::dom::document::Document;
@ -158,7 +158,7 @@ impl FrameHolder {
pub struct MediaFrameRenderer {
player_id: Option<u64>,
compositor_api: CrossProcessCompositorApi,
current_frame: Option<(ImageKey, i32, i32)>,
current_frame: Option<MediaFrame>,
old_frame: Option<ImageKey>,
very_old_frame: Option<ImageKey>,
current_frame_holder: Option<FrameHolder>,
@ -179,8 +179,12 @@ impl MediaFrameRenderer {
}
fn render_poster_frame(&mut self, image: Arc<Image>) {
if let Some(image_id) = image.id {
self.current_frame = Some((image_id, image.width as i32, image.height as i32));
if let Some(image_key) = image.id {
self.current_frame = Some(MediaFrame {
image_key,
width: image.width as i32,
height: image.height as i32,
});
self.show_poster = true;
}
}
@ -206,13 +210,14 @@ impl VideoFrameRenderer for MediaFrameRenderer {
ImageDescriptorFlags::empty(),
);
match self.current_frame {
Some((ref image_key, ref mut width, ref mut height))
if *width == frame.get_width() && *height == frame.get_height() =>
match &mut self.current_frame {
Some(ref mut current_frame)
if current_frame.width == frame.get_width() &&
current_frame.height == frame.get_height() =>
{
if !frame.is_gl_texture() {
updates.push(ImageUpdate::UpdateImage(
*image_key,
current_frame.image_key,
descriptor,
SerializableImageData::Raw(IpcSharedMemory::from_bytes(&frame.get_data())),
));
@ -226,17 +231,17 @@ impl VideoFrameRenderer for MediaFrameRenderer {
updates.push(ImageUpdate::DeleteImage(old_image_key));
}
},
Some((ref mut image_key, ref mut width, ref mut height)) => {
self.old_frame = Some(*image_key);
Some(ref mut current_frame) => {
self.old_frame = Some(current_frame.image_key);
let Some(new_image_key) = self.compositor_api.generate_image_key() else {
return;
};
/* update current_frame */
*image_key = new_image_key;
*width = frame.get_width();
*height = frame.get_height();
current_frame.image_key = new_image_key;
current_frame.width = frame.get_width();
current_frame.height = frame.get_height();
let image_data = if frame.is_gl_texture() && self.player_id.is_some() {
let texture_target = if frame.is_external_oes() {
@ -264,7 +269,12 @@ impl VideoFrameRenderer for MediaFrameRenderer {
let Some(image_key) = self.compositor_api.generate_image_key() else {
return;
};
self.current_frame = Some((image_key, frame.get_width(), frame.get_height()));
self.current_frame = Some(MediaFrame {
image_key,
width: frame.get_width(),
height: frame.get_height(),
});
let image_data = if frame.is_gl_texture() && self.player_id.is_some() {
let texture_target = if frame.is_external_oes() {
@ -1320,6 +1330,7 @@ impl HTMLMediaElement {
}
// Step 6.
self.handle_resize(Some(image.width), Some(image.height));
self.video_renderer
.lock()
.unwrap()
@ -1584,6 +1595,10 @@ impl HTMLMediaElement {
},
PlayerEvent::VideoFrameUpdated => {
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
// Check if the frame was resized
if let Some(frame) = self.video_renderer.lock().unwrap().current_frame {
self.handle_resize(Some(frame.width as u32), Some(frame.height as u32));
}
},
PlayerEvent::MetadataUpdated(ref metadata) => {
// https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list
@ -1727,18 +1742,7 @@ impl HTMLMediaElement {
}
// Step 5.
if self.is::<HTMLVideoElement>() {
let video_elem = self.downcast::<HTMLVideoElement>().unwrap();
if video_elem.get_video_width() != metadata.width ||
video_elem.get_video_height() != metadata.height
{
video_elem.set_video_width(metadata.width);
video_elem.set_video_height(metadata.height);
let window = window_from_node(self);
let task_source = window.task_manager().media_element_task_source();
task_source.queue_simple_event(self.upcast(), atom!("resize"), &window);
}
}
self.handle_resize(Some(metadata.width), Some(metadata.height));
// Step 6.
self.change_ready_state(ReadyState::HaveMetadata);
@ -1971,6 +1975,21 @@ impl HTMLMediaElement {
.map(|holder| holder.get_frame())
}
pub fn get_current_frame_data(&self) -> Option<MediaFrame> {
self.video_renderer.lock().unwrap().current_frame
}
pub fn clear_current_frame_data(&self) {
self.handle_resize(None, None);
self.video_renderer.lock().unwrap().current_frame = None
}
fn handle_resize(&self, width: Option<u32>, height: Option<u32>) {
if let Some(video_elem) = self.downcast::<HTMLVideoElement>() {
video_elem.resize(width, height);
}
}
/// By default the audio is rendered through the audio sink automatically
/// selected by the servo-media Player instance. However, in some cases, like
/// the WebAudio MediaElementAudioSourceNode, we need to set a custom audio
@ -1998,13 +2017,11 @@ impl HTMLMediaElement {
self.duration.set(duration);
}
/// Sets a new value for the show_poster propperty. If the poster is being hidden
/// because new frames should render, updates video_renderer to allow it.
fn set_show_poster(&self, show_poster: bool) {
/// Sets a new value for the show_poster propperty. Updates video_rederer
/// with the new value.
pub fn set_show_poster(&self, show_poster: bool) {
self.show_poster.set(show_poster);
if !show_poster {
self.video_renderer.lock().unwrap().show_poster = false;
}
self.video_renderer.lock().unwrap().show_poster = show_poster;
}
pub fn reset(&self) {
@ -2478,6 +2495,7 @@ impl VirtualMethods for HTMLMediaElement {
},
local_name!("src") => {
if mutation.new_value(attr).is_none() {
self.clear_current_frame_data();
return;
}
self.media_element_load_algorithm(CanGc::note());
@ -2508,23 +2526,6 @@ impl VirtualMethods for HTMLMediaElement {
}
}
pub trait LayoutHTMLMediaElementHelpers {
fn data(self) -> HTMLMediaData;
}
impl LayoutHTMLMediaElementHelpers for LayoutDom<'_, HTMLMediaElement> {
fn data(self) -> HTMLMediaData {
HTMLMediaData {
current_frame: self
.unsafe_get()
.video_renderer
.lock()
.unwrap()
.current_frame,
}
}
}
#[derive(JSTraceable, MallocSizeOf)]
pub enum MediaElementMicrotask {
ResourceSelection {

View file

@ -7,7 +7,7 @@ use std::sync::Arc;
use dom_struct::dom_struct;
use euclid::default::Size2D;
use html5ever::{local_name, LocalName, Prefix};
use html5ever::{local_name, namespace_url, ns, LocalName, Prefix};
use ipc_channel::ipc;
use js::rust::HandleObject;
use net_traits::image_cache::{
@ -19,8 +19,10 @@ use net_traits::{
FetchMetadata, FetchResponseListener, FetchResponseMsg, NetworkError, ResourceFetchTiming,
ResourceTimingType,
};
use script_layout_interface::{HTMLMediaData, MediaMetadata};
use servo_media::player::video::VideoFrame;
use servo_url::ServoUrl;
use style::attr::{AttrValue, LengthOrPercentageOrAuto};
use crate::document_loader::{LoadBlocker, LoadType};
use crate::dom::attr::Attr;
@ -29,10 +31,10 @@ use crate::dom::bindings::codegen::Bindings::HTMLVideoElementBinding::HTMLVideoE
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::DomRoot;
use crate::dom::bindings::root::{DomRoot, LayoutDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::document::Document;
use crate::dom::element::{AttributeMutation, Element};
use crate::dom::element::{AttributeMutation, Element, LayoutElementHelpers};
use crate::dom::globalscope::GlobalScope;
use crate::dom::htmlmediaelement::{HTMLMediaElement, ReadyState};
use crate::dom::node::{document_from_node, window_from_node, Node};
@ -43,16 +45,13 @@ use crate::image_listener::{generate_cache_listener_for_element, ImageCacheListe
use crate::network_listener::{self, PreInvoke, ResourceTimingListener};
use crate::script_runtime::CanGc;
const DEFAULT_WIDTH: u32 = 300;
const DEFAULT_HEIGHT: u32 = 150;
#[dom_struct]
pub struct HTMLVideoElement {
htmlmediaelement: HTMLMediaElement,
/// <https://html.spec.whatwg.org/multipage/#dom-video-videowidth>
video_width: Cell<u32>,
video_width: Cell<Option<u32>>,
/// <https://html.spec.whatwg.org/multipage/#dom-video-videoheight>
video_height: Cell<u32>,
video_height: Cell<Option<u32>>,
/// Incremented whenever tasks associated with this element are cancelled.
generation_id: Cell<u32>,
/// Poster frame fetch request canceller.
@ -64,6 +63,8 @@ pub struct HTMLVideoElement {
#[ignore_malloc_size_of = "VideoFrame"]
#[no_trace]
last_frame: DomRefCell<Option<VideoFrame>>,
/// Indicates if it has already sent a resize event for a given size
sent_resize: Cell<Option<(u32, u32)>>,
}
impl HTMLVideoElement {
@ -74,12 +75,13 @@ impl HTMLVideoElement {
) -> HTMLVideoElement {
HTMLVideoElement {
htmlmediaelement: HTMLMediaElement::new_inherited(local_name, prefix, document),
video_width: Cell::new(DEFAULT_WIDTH),
video_height: Cell::new(DEFAULT_HEIGHT),
video_width: Cell::new(None),
video_height: Cell::new(None),
generation_id: Cell::new(0),
poster_frame_canceller: DomRefCell::new(Default::default()),
load_blocker: Default::default(),
last_frame: Default::default(),
sent_resize: Cell::new(None),
}
}
@ -101,20 +103,36 @@ impl HTMLVideoElement {
)
}
pub fn get_video_width(&self) -> u32 {
pub fn get_video_width(&self) -> Option<u32> {
self.video_width.get()
}
pub fn set_video_width(&self, width: u32) {
self.video_width.set(width);
}
pub fn get_video_height(&self) -> u32 {
pub fn get_video_height(&self) -> Option<u32> {
self.video_height.get()
}
pub fn set_video_height(&self, height: u32) {
/// <https://html.spec.whatwg.org/multipage#event-media-resize>
pub fn resize(&self, width: Option<u32>, height: Option<u32>) -> Option<(u32, u32)> {
self.video_width.set(width);
self.video_height.set(height);
let width = width?;
let height = height?;
if self.sent_resize.get() == Some((width, height)) {
return None;
}
let sent_resize = if self.htmlmediaelement.get_ready_state() == ReadyState::HaveNothing {
None
} else {
let window = window_from_node(self);
let task_source = window.task_manager().media_element_task_source();
task_source.queue_simple_event(self.upcast(), atom!("resize"), &window);
Some((width, height))
};
self.sent_resize.set(sent_resize);
sent_resize
}
pub fn get_current_frame_data(&self) -> Option<(Option<ipc::IpcSharedMemory>, Size2D<u32>)> {
@ -228,7 +246,7 @@ impl HTMLVideoElementMethods for HTMLVideoElement {
if self.htmlmediaelement.get_ready_state() == ReadyState::HaveNothing {
return 0;
}
self.video_width.get()
self.video_width.get().unwrap_or(0)
}
// https://html.spec.whatwg.org/multipage/#dom-video-videoheight
@ -236,7 +254,7 @@ impl HTMLVideoElementMethods for HTMLVideoElement {
if self.htmlmediaelement.get_ready_state() == ReadyState::HaveNothing {
return 0;
}
self.video_height.get()
self.video_height.get().unwrap_or(0)
}
// https://html.spec.whatwg.org/multipage/#dom-video-poster
@ -258,10 +276,25 @@ impl VirtualMethods for HTMLVideoElement {
fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
self.super_type().unwrap().attribute_mutated(attr, mutation);
if let Some(new_value) = mutation.new_value(attr) {
if attr.local_name() == &local_name!("poster") {
self.fetch_poster_frame(&new_value, CanGc::note());
if attr.local_name() == &local_name!("poster") {
if let Some(new_value) = mutation.new_value(attr) {
self.fetch_poster_frame(&new_value, CanGc::note())
} else {
self.htmlmediaelement.clear_current_frame_data();
self.htmlmediaelement.set_show_poster(false);
}
};
}
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("width") | &local_name!("height") => {
AttrValue::from_dimension(value.into())
},
_ => self
.super_type()
.unwrap()
.parse_plain_attribute(name, value),
}
}
}
@ -408,3 +441,55 @@ impl PosterFrameFetchContext {
}
}
}
pub trait LayoutHTMLVideoElementHelpers {
fn data(self) -> HTMLMediaData;
fn get_width(self) -> LengthOrPercentageOrAuto;
fn get_height(self) -> LengthOrPercentageOrAuto;
}
impl LayoutDom<'_, HTMLVideoElement> {
fn width_attr(self) -> Option<LengthOrPercentageOrAuto> {
self.upcast::<Element>()
.get_attr_for_layout(&ns!(), &local_name!("width"))
.map(AttrValue::as_dimension)
.cloned()
}
fn height_attr(self) -> Option<LengthOrPercentageOrAuto> {
self.upcast::<Element>()
.get_attr_for_layout(&ns!(), &local_name!("height"))
.map(AttrValue::as_dimension)
.cloned()
}
}
impl LayoutHTMLVideoElementHelpers for LayoutDom<'_, HTMLVideoElement> {
fn data(self) -> HTMLMediaData {
let video = self.unsafe_get();
// Get the current frame being rendered.
let current_frame = video.htmlmediaelement.get_current_frame_data();
// This value represents the natural width and height of the video.
// It may exist even if there is no current frame (for example, after the
// metadata of the video is loaded).
let metadata = video
.get_video_width()
.zip(video.get_video_height())
.map(|(width, height)| MediaMetadata { width, height });
HTMLMediaData {
current_frame,
metadata,
}
}
fn get_width(self) -> LengthOrPercentageOrAuto {
self.width_attr().unwrap_or(LengthOrPercentageOrAuto::Auto)
}
fn get_height(self) -> LengthOrPercentageOrAuto {
self.height_attr().unwrap_or(LengthOrPercentageOrAuto::Auto)
}
}

View file

@ -88,9 +88,9 @@ use crate::dom::htmliframeelement::{HTMLIFrameElement, HTMLIFrameElementLayoutMe
use crate::dom::htmlimageelement::{HTMLImageElement, LayoutHTMLImageElementHelpers};
use crate::dom::htmlinputelement::{HTMLInputElement, LayoutHTMLInputElementHelpers};
use crate::dom::htmllinkelement::HTMLLinkElement;
use crate::dom::htmlmediaelement::{HTMLMediaElement, LayoutHTMLMediaElementHelpers};
use crate::dom::htmlstyleelement::HTMLStyleElement;
use crate::dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaElementHelpers};
use crate::dom::htmlvideoelement::{HTMLVideoElement, LayoutHTMLVideoElementHelpers};
use crate::dom::mouseevent::MouseEvent;
use crate::dom::mutationobserver::{Mutation, MutationObserver, RegisteredObserver};
use crate::dom::nodelist::NodeList;
@ -1553,7 +1553,7 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
}
fn media_data(self) -> Option<HTMLMediaData> {
self.downcast::<HTMLMediaElement>()
self.downcast::<HTMLVideoElement>()
.map(|media| media.data())
}