Skip to content

Commit 07cf9a9

Browse files
mukilanmrobinson
andauthored
compositor: Rename RenderingGroupId to PainterId (servo#40307)
And let the Compositor create the `PainterId` when it creates a new `Painter`. Also make inline formatting code take `LayoutContext` rather than threading it via the `InlineFormattingContextBuilder`. Testing: Should preserve existing behavior, so covered by existing tests. Signed-off-by: Mukilan Thiyagarajan <[email protected]> Co-authored-by: Martin Robinson <[email protected]>
1 parent ea5929e commit 07cf9a9

File tree

19 files changed

+146
-181
lines changed

19 files changed

+146
-181
lines changed

components/compositing/compositor.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::sync::{Arc, Mutex};
1010
use std::time::{SystemTime, UNIX_EPOCH};
1111

1212
use base::generic_channel::RoutedReceiver;
13-
use base::id::WebViewId;
13+
use base::id::{PainterId, WebViewId};
1414
use bitflags::bitflags;
1515
use canvas_traits::webgl::WebGLThreads;
1616
use compositing_traits::{CompositorMsg, WebRenderExternalImageRegistry, WebViewTrait};
@@ -132,6 +132,10 @@ impl IOCompositor {
132132
self.painters[0].borrow_mut()
133133
}
134134

135+
pub fn painter_id(&self) -> PainterId {
136+
self.painters[0].borrow().painter_id
137+
}
138+
135139
pub fn deinit(&mut self) {
136140
for painter in &self.painters {
137141
painter.borrow_mut().deinit();
@@ -330,7 +334,7 @@ impl IOCompositor {
330334
number_of_font_keys,
331335
number_of_font_instance_keys,
332336
result_sender,
333-
_rendering_group_id,
337+
_painter_id,
334338
) => {
335339
let _ = result_sender.send(
336340
self.painter_mut()
@@ -419,7 +423,7 @@ impl IOCompositor {
419423
number_of_font_keys,
420424
number_of_font_instance_keys,
421425
result_sender,
422-
_rendering_group_id,
426+
_painter_id,
423427
) => {
424428
let _ = result_sender.send(
425429
self.painter_mut()

components/compositing/painter.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use std::sync::{Arc, Mutex};
1111

1212
use base::Epoch;
1313
use base::cross_process_instant::CrossProcessInstant;
14-
use base::id::{PipelineId, WebViewId};
14+
use base::id::{PainterId, PipelineId, WebViewId};
1515
use canvas_traits::webgl::{GlType, WebGLThreads};
1616
use compositing_traits::display_list::{CompositorDisplayListInfo, ScrollType};
1717
use compositing_traits::largest_contentful_paint_candidate::LCPCandidate;
@@ -81,6 +81,9 @@ pub(crate) struct Painter {
8181
/// The [`RenderingContext`] instance that webrender targets, which is the viewport.
8282
pub(crate) rendering_context: Rc<dyn RenderingContext>,
8383

84+
/// The ID of this painter.
85+
pub(crate) painter_id: PainterId,
86+
8487
/// Our [`WebViewRenderer`]s, one for every `WebView`.
8588
pub(crate) webview_renderers: WebViewManager<WebViewRenderer>,
8689

@@ -289,6 +292,7 @@ impl Painter {
289292
info!("Running on {gl_renderer} with OpenGL version {gl_version}");
290293

291294
let painter = Painter {
295+
painter_id: PainterId::next(),
292296
embedder_to_constellation_sender,
293297
webview_renderers: Default::default(),
294298
rendering_context,

components/compositing/webview_manager.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ impl WebViewManager<WebViewRenderer> {
122122

123123
#[cfg(test)]
124124
mod test {
125-
use base::id::{BrowsingContextId, Index, PipelineNamespace, PipelineNamespaceId, WebViewId};
125+
use base::id::{
126+
BrowsingContextId, Index, PipelineNamespace, PipelineNamespaceId, TEST_PAINTER_ID,
127+
WebViewId,
128+
};
126129

127130
use crate::webview_manager::WebViewManager;
128131
use crate::webview_renderer::UnknownWebView;
@@ -150,9 +153,10 @@ mod test {
150153
let mut webviews = WebViewManager::default();
151154

152155
// entry() adds the webview to the map, but not the painting order.
153-
webviews.entry(WebViewId::new()).or_insert('a');
154-
webviews.entry(WebViewId::new()).or_insert('b');
155-
webviews.entry(WebViewId::new()).or_insert('c');
156+
let painter_id = TEST_PAINTER_ID;
157+
webviews.entry(WebViewId::new(painter_id)).or_insert('a');
158+
webviews.entry(WebViewId::new(painter_id)).or_insert('b');
159+
webviews.entry(WebViewId::new(painter_id)).or_insert('c');
156160
assert!(webviews.get(webview_id(0, 1)).is_some());
157161
assert!(webviews.get(webview_id(0, 2)).is_some());
158162
assert!(webviews.get(webview_id(0, 3)).is_some());

components/constellation/webview_manager.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ impl<WebView> WebViewManager<WebView> {
8383

8484
#[cfg(test)]
8585
mod test {
86-
use base::id::{BrowsingContextId, Index, PipelineNamespace, PipelineNamespaceId, WebViewId};
86+
use base::id::{
87+
BrowsingContextId, Index, PipelineNamespace, PipelineNamespaceId, TEST_PAINTER_ID,
88+
WebViewId,
89+
};
8790

8891
use crate::webview_manager::WebViewManager;
8992

@@ -119,9 +122,9 @@ mod test {
119122
let mut webviews = WebViewManager::default();
120123

121124
// add() adds the webview to the map, but does not focus it.
122-
webviews.add(WebViewId::new(), 'a');
123-
webviews.add(WebViewId::new(), 'b');
124-
webviews.add(WebViewId::new(), 'c');
125+
webviews.add(WebViewId::new(TEST_PAINTER_ID), 'a');
126+
webviews.add(WebViewId::new(TEST_PAINTER_ID), 'b');
127+
webviews.add(WebViewId::new(TEST_PAINTER_ID), 'c');
125128
assert_eq!(
126129
webviews_sorted(&webviews),
127130
vec![(id(0, 1), 'a'), (id(0, 2), 'b'), (id(0, 3), 'c'),]

components/devtools/id.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ pub(crate) fn test_id_map() {
158158

159159
test_sequential_id_assignment!(
160160
DevtoolsBrowserId,
161-
|| WebViewId::new(),
161+
|| WebViewId::new(base::id::TEST_PAINTER_ID),
162162
|id_map: &mut IdMap, id| id_map.browser_id(id)
163163
);
164164
test_sequential_id_assignment!(

components/fonts/font.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::time::Instant;
1212
use std::{iter, str};
1313

1414
use app_units::Au;
15-
use base::id::RenderingGroupId;
15+
use base::id::PainterId;
1616
use bitflags::bitflags;
1717
use euclid::default::{Point2D, Rect};
1818
use euclid::num::Zero;
@@ -232,7 +232,7 @@ pub struct Font {
232232

233233
shaper: OnceLock<Shaper>,
234234
cached_shape_data: RwLock<CachedShapeData>,
235-
font_instance_key: RwLock<FxHashMap<RenderingGroupId, FontInstanceKey>>,
235+
font_instance_key: RwLock<FxHashMap<PainterId, FontInstanceKey>>,
236236

237237
/// If this is a synthesized small caps font, then this font reference is for
238238
/// the version of the font used to replace lowercase ASCII letters. It's up
@@ -326,16 +326,12 @@ impl Font {
326326
})
327327
}
328328

329-
pub fn key(
330-
&self,
331-
rendering_group_id: RenderingGroupId,
332-
font_context: &FontContext,
333-
) -> FontInstanceKey {
329+
pub fn key(&self, painter_id: PainterId, font_context: &FontContext) -> FontInstanceKey {
334330
*self
335331
.font_instance_key
336332
.write()
337-
.entry(rendering_group_id)
338-
.or_insert_with(|| font_context.create_font_instance_key(self, rendering_group_id))
333+
.entry(painter_id)
334+
.or_insert_with(|| font_context.create_font_instance_key(self, painter_id))
339335
}
340336

341337
/// Return the data for this `Font`. Note that this is currently highly inefficient for system

components/fonts/font_context.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::sync::Arc;
1010
use std::sync::atomic::{AtomicBool, Ordering};
1111

1212
use app_units::Au;
13-
use base::id::{RenderingGroupId, WebViewId};
13+
use base::id::{PainterId, WebViewId};
1414
use compositing_traits::CrossProcessCompositorApi;
1515
use fonts_traits::{
1616
CSSFontFaceDescriptors, FontDescriptor, FontIdentifier, FontTemplate, FontTemplateRef,
@@ -286,22 +286,22 @@ impl FontContext {
286286
pub(crate) fn create_font_instance_key(
287287
&self,
288288
font: &Font,
289-
rendering_group_id: RenderingGroupId,
289+
painter_id: PainterId,
290290
) -> FontInstanceKey {
291291
match font.template.identifier() {
292292
FontIdentifier::Local(_) => self.system_font_service_proxy.get_system_font_instance(
293293
font.template.identifier(),
294294
font.descriptor.pt_size,
295295
font.webrender_font_instance_flags(),
296296
font.variations().to_owned(),
297-
rendering_group_id,
297+
painter_id,
298298
),
299299
FontIdentifier::Web(_) => self.create_web_font_instance(
300300
font.template.clone(),
301301
font.descriptor.pt_size,
302302
font.webrender_font_instance_flags(),
303303
font.variations().to_owned(),
304-
rendering_group_id,
304+
painter_id,
305305
),
306306
}
307307
}
@@ -312,7 +312,7 @@ impl FontContext {
312312
pt_size: Au,
313313
flags: FontInstanceFlags,
314314
variations: Vec<FontVariation>,
315-
rendering_group_id: RenderingGroupId,
315+
painter_id: PainterId,
316316
) -> FontInstanceKey {
317317
let identifier = font_template.identifier().clone();
318318
let font_data = self
@@ -323,9 +323,7 @@ impl FontContext {
323323
.write()
324324
.entry(identifier.clone())
325325
.or_insert_with(|| {
326-
let font_key = self
327-
.system_font_service_proxy
328-
.generate_font_key(rendering_group_id);
326+
let font_key = self.system_font_service_proxy.generate_font_key(painter_id);
329327
self.compositor_api.lock().add_font(
330328
font_key,
331329
font_data.as_ipc_shared_memory(),
@@ -347,7 +345,7 @@ impl FontContext {
347345
.or_insert_with(|| {
348346
let font_instance_key = self
349347
.system_font_service_proxy
350-
.generate_font_instance_key(rendering_group_id);
348+
.generate_font_instance_key(painter_id);
351349
self.compositor_api.lock().add_font_instance(
352350
font_instance_key,
353351
font_key,

0 commit comments

Comments
 (0)