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

Add external event notifications in transactions. #3020

Merged
merged 3 commits into from Sep 12, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Add external event notifications in transactions.

  • Loading branch information
nical committed Sep 11, 2018
commit 0e018f14458b33fd273e2adc98474e2ab0590eb4
@@ -575,6 +575,7 @@ impl RenderBackend {
txn.document_id,
replace(&mut txn.resource_updates, Vec::new()),
replace(&mut txn.frame_ops, Vec::new()),
replace(&mut txn.notifications, Vec::new()),
txn.build_frame,
txn.render_frame,
&mut frame_counter,
@@ -802,6 +803,7 @@ impl RenderBackend {
resource_updates: transaction_msg.resource_updates,
frame_ops: transaction_msg.frame_ops,
rasterized_blobs: Vec::new(),
notifications: Vec::new(),
set_root_pipeline: None,
build_frame: transaction_msg.generate_frame,
render_frame: transaction_msg.generate_frame,
@@ -837,6 +839,7 @@ impl RenderBackend {
txn.document_id,
replace(&mut txn.resource_updates, Vec::new()),
replace(&mut txn.frame_ops, Vec::new()),
replace(&mut txn.notifications, Vec::new()),
txn.build_frame,
txn.render_frame,
frame_counter,
@@ -873,6 +876,7 @@ impl RenderBackend {
document_id: DocumentId,
resource_updates: Vec<ResourceUpdate>,
mut frame_ops: Vec<FrameMsg>,
notifications: Vec<ExternalEvent>,
mut build_frame: bool,
mut render_frame: bool,
frame_counter: &mut u32,
@@ -982,6 +986,10 @@ impl RenderBackend {
self.result_tx.send(msg).unwrap();
}

for evt in notifications {
self.notifier.external_event(evt)
}

// Always forward the transaction to the renderer if a frame was requested,
// otherwise gecko can get into a state where it waits (forever) for the
// transaction to complete before sending new work.
@@ -4,7 +4,7 @@

use api::{AsyncBlobImageRasterizer, BlobImageRequest, BlobImageParams, BlobImageResult};
use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, ResourceUpdate, Epoch};
use api::{BuiltDisplayList, ColorF, LayoutSize};
use api::{BuiltDisplayList, ColorF, LayoutSize, ExternalEvent};
use api::channel::MsgSender;
use frame_builder::{FrameBuilderConfig, FrameBuilder};
use clip_scroll_tree::ClipScrollTree;
@@ -30,6 +30,7 @@ pub struct Transaction {
pub rasterized_blobs: Vec<(BlobImageRequest, BlobImageResult)>,
pub resource_updates: Vec<ResourceUpdate>,
pub frame_ops: Vec<FrameMsg>,
pub notifications: Vec<ExternalEvent>,
pub set_root_pipeline: Option<PipelineId>,
pub build_frame: bool,
pub render_frame: bool,
@@ -61,6 +62,7 @@ pub struct BuiltTransaction {
pub blob_rasterizer: Option<Box<AsyncBlobImageRasterizer>>,
pub frame_ops: Vec<FrameMsg>,
pub removed_pipelines: Vec<PipelineId>,
pub notifications: Vec<ExternalEvent>,
pub scene_build_start_time: u64,
pub scene_build_end_time: u64,
pub build_frame: bool,
@@ -251,6 +253,7 @@ impl SceneBuilder {
blob_rasterizer: None,
frame_ops: Vec::new(),
removed_pipelines: Vec::new(),
notifications: Vec::new(),
scene_build_start_time,
scene_build_end_time: precise_time_ns(),
});
@@ -332,6 +335,7 @@ impl SceneBuilder {
blob_rasterizer: replace(&mut txn.blob_rasterizer, None),
frame_ops: replace(&mut txn.frame_ops, Vec::new()),
removed_pipelines: replace(&mut txn.removed_pipelines, Vec::new()),
notifications: replace(&mut txn.notifications, Vec::new()),
scene_build_start_time,
scene_build_end_time: precise_time_ns(),
})
@@ -48,6 +48,8 @@ pub struct Transaction {
// Additional display list data.
payloads: Vec<Payload>,

notifications: Vec<ExternalEvent>,

// Resource updates are applied after scene building.
pub resource_updates: Vec<ResourceUpdate>,

@@ -67,6 +69,7 @@ impl Transaction {
frame_ops: Vec::new(),
resource_updates: Vec::new(),
payloads: Vec::new(),
notifications: Vec::new(),
use_scene_builder_thread: true,
generate_frame: false,
low_priority: false,
@@ -88,7 +91,8 @@ impl Transaction {
!self.generate_frame &&
self.scene_ops.is_empty() &&
self.frame_ops.is_empty() &&
self.resource_updates.is_empty()
self.resource_updates.is_empty() &&
self.notifications.is_empty()
}

pub fn update_epoch(&mut self, pipeline_id: PipelineId, epoch: Epoch) {
@@ -171,6 +175,18 @@ impl Transaction {
self.merge(resources);
}

// Note: Gecko uses this to get notified when a transaction that contains
// potentially long blob rasterization or scene build is ready to be rendered.
// so that the tab-switching integration can react adequately when tab
// switching takes too long. For this use case what matters is that the
// notification doesn't fire before scene building and blob rasterization.

/// Trigger the external event notification when the transaction gets passed
/// the frame building stage.
pub fn notify(&mut self, event: ExternalEvent) {

This comment has been minimized.

@kvark

kvark Sep 5, 2018

Member

can we have some documentation here on what does this mean? When will the notification be fired, and why we need it.

self.notifications.push(event);
}

pub fn set_window_parameters(
&mut self,
window_size: DeviceUintSize,
@@ -257,6 +273,7 @@ impl Transaction {
scene_ops: self.scene_ops,
frame_ops: self.frame_ops,
resource_updates: self.resource_updates,
notifications: self.notifications,
use_scene_builder_thread: self.use_scene_builder_thread,
generate_frame: self.generate_frame,
low_priority: self.low_priority,
@@ -368,6 +385,7 @@ pub struct TransactionMsg {
pub scene_ops: Vec<SceneMsg>,
pub frame_ops: Vec<FrameMsg>,
pub resource_updates: Vec<ResourceUpdate>,
pub notifications: Vec<ExternalEvent>,
pub generate_frame: bool,
pub use_scene_builder_thread: bool,
pub low_priority: bool,
@@ -378,7 +396,8 @@ impl TransactionMsg {
!self.generate_frame &&
self.scene_ops.is_empty() &&
self.frame_ops.is_empty() &&
self.resource_updates.is_empty()
self.resource_updates.is_empty() &&
self.notifications.is_empty()
}

// TODO: We only need this for a few RenderApi methods which we should remove.
@@ -387,6 +406,7 @@ impl TransactionMsg {
scene_ops: Vec::new(),
frame_ops: vec![msg],
resource_updates: Vec::new(),
notifications: Vec::new(),
generate_frame: false,
use_scene_builder_thread: false,
low_priority: false,
@@ -398,6 +418,7 @@ impl TransactionMsg {
scene_ops: vec![msg],
frame_ops: Vec::new(),
resource_updates: Vec::new(),
notifications: Vec::new(),
generate_frame: false,
use_scene_builder_thread: false,
low_priority: false,
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.