Auto merge of #24525 - glowe:issue-23009/separate_angle_and_disable_vsync, r=jdm

Issue 23009/separate angle and disable vsync

The `--angle` and `--disable-vsync` options were declared as global options, but only used in the Glutin embedding for desktop builds. Moving them to the Glutin embedding code makes them easier to update in the future.

I modified `opts::from_cmdline_args` to accept a `getopts::Options` (as prescribed in the issue) and augmented `opts::ArgumentParsingResult` to include an `opts::Matches` and `content-process` String when appropriate. I could use some feedback on this last bit. I could have changed the function to return `opts::Matches` and have the embedding code look for the presence of `content-process`, but I felt that the approach I went with was closer to the original design.

The other aspect I'm not sure about is moving `disable-vsync` from a global debug option to a plain embedder option. This changes the command line interface for glutin, which is maybe bad. However I wasn't sure whether it was worth preserving the original behavior given the complexity of injecting debug options into `opts::from_cmdline_args`.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes partially fix #23009 – there are 2 more options to deal with, but I'm not sure if we should handle them yet.
- [x] These changes do not require tests because this is a refactoring and I'm hoping that the existing tests cover these changes.

r? @jdm
This commit is contained in:
bors-servo 2019-10-26 14:58:54 -04:00 committed by GitHub
commit 2ad6e94091
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 180 additions and 127 deletions

View file

@ -7,7 +7,7 @@
use crate::prefs::{self, PrefValue};
use euclid::Size2D;
use getopts::Options;
use getopts::{Matches, Options};
use servo_geometry::DeviceIndependentPixel;
use servo_url::ServoUrl;
use std::borrow::Cow;
@ -32,10 +32,6 @@ pub struct Opts {
/// The maximum size of each tile in pixels (`-s`).
pub tile_size: usize,
/// The ratio of device pixels per px at the default scale. If unspecified, will use the
/// platform default setting.
pub device_pixels_per_px: Option<f32>,
/// `None` to disable the time profiler or `Some` to enable it with:
///
/// - an interval in seconds to cause it to produce output on that interval.
@ -78,9 +74,6 @@ pub struct Opts {
pub headless: bool,
/// Use ANGLE to create the GL context (Windows-only).
pub angle: bool,
/// True to exit on thread failure instead of displaying about:failure.
pub hard_fail: bool,
@ -184,12 +177,6 @@ pub struct Opts {
/// True to exit after the page load (`-x`).
pub exit_after_load: bool,
/// Do not use native titlebar
pub no_native_titlebar: bool,
/// Enable vsync in the compositor
pub enable_vsync: bool,
/// True to show webrender profiling stats on screen.
pub webrender_stats: bool,
@ -207,9 +194,6 @@ pub struct Opts {
/// after each change is made.
pub precache_shaders: bool,
/// True if WebRender should use multisample antialiasing.
pub use_msaa: bool,
/// Directory for a default config directory
pub config_dir: Option<PathBuf>,
@ -231,9 +215,6 @@ pub struct Opts {
/// Print Progressive Web Metrics to console.
pub print_pwm: bool,
/// Only shutdown once all theads are finished.
pub clean_shutdown: bool,
}
fn print_usage(app: &str, opts: &Options) {
@ -314,9 +295,6 @@ pub struct DebugOptions {
/// Load web fonts synchronously to avoid non-deterministic network-driven reflows.
pub load_webfonts_synchronously: bool,
/// Disable vsync in the compositor
pub disable_vsync: bool,
/// Show webrender profiling stats on screen.
pub webrender_stats: bool,
@ -326,9 +304,6 @@ pub struct DebugOptions {
/// Enable webrender instanced draw call batching.
pub webrender_disable_batch: bool,
/// Use multisample antialiasing in WebRender.
pub use_msaa: bool,
// don't skip any backtraces on panic
pub full_backtraces: bool,
@ -368,11 +343,9 @@ impl DebugOptions {
"replace-surrogates" => self.replace_surrogates = true,
"gc-profile" => self.gc_profile = true,
"load-webfonts-synchronously" => self.load_webfonts_synchronously = true,
"disable-vsync" => self.disable_vsync = true,
"wr-stats" => self.webrender_stats = true,
"wr-record" => self.webrender_record = true,
"wr-no-batch" => self.webrender_disable_batch = true,
"msaa" => self.use_msaa = true,
"full-backtraces" => self.full_backtraces = true,
"precache-shaders" => self.precache_shaders = true,
"signpost" => self.signpost = true,
@ -462,12 +435,7 @@ fn print_debug_usage(app: &str) -> ! {
"load-webfonts-synchronously",
"Load web fonts synchronously to avoid non-deterministic network-driven reflows",
);
print_option(
"disable-vsync",
"Disable vsync mode in the compositor to allow profiling at more than monitor refresh rate",
);
print_option("wr-stats", "Show WebRender profiler on screen.");
print_option("msaa", "Use multisample antialiasing in WebRender.");
print_option("full-backtraces", "Print full backtraces for all errors");
print_option("wr-debug", "Display webrender tile borders.");
print_option("wr-no-batch", "Disable webrender instanced batching.");
@ -552,7 +520,6 @@ pub fn default_opts() -> Opts {
is_running_problem_test: false,
url: None,
tile_size: 512,
device_pixels_per_px: None,
time_profiling: None,
time_profiler_trace_path: None,
mem_profiler_period: None,
@ -564,7 +531,6 @@ pub fn default_opts() -> Opts {
gc_profile: false,
load_webfonts_synchronously: false,
headless: false,
angle: false,
hard_fail: true,
bubble_inline_sizes_separately: false,
show_debug_fragment_borders: false,
@ -594,10 +560,7 @@ pub fn default_opts() -> Opts {
style_sharing_stats: false,
convert_mouse_to_touch: false,
exit_after_load: false,
no_native_titlebar: false,
enable_vsync: true,
webrender_stats: false,
use_msaa: false,
config_dir: None,
full_backtraces: false,
is_printing_version: false,
@ -609,19 +572,16 @@ pub fn default_opts() -> Opts {
certificate_path: None,
unminify_js: false,
print_pwm: false,
clean_shutdown: false,
}
}
pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
pub fn from_cmdline_args(mut opts: Options, args: &[String]) -> ArgumentParsingResult {
let (app_name, args) = args.split_first().unwrap();
let mut opts = Options::new();
opts.optflag("c", "cpu", "CPU painting");
opts.optflag("g", "gpu", "GPU painting");
opts.optopt("o", "output", "Output file", "output.png");
opts.optopt("s", "size", "Size of tiles", "512");
opts.optopt("", "device-pixel-ratio", "Device pixels per px", "");
opts.optflagopt(
"p",
"profile",
@ -673,11 +633,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
"",
);
opts.optflag("z", "headless", "Headless mode");
opts.optflag(
"",
"angle",
"Use ANGLE to create a GL context (Windows-only)",
);
opts.optflag(
"f",
"hard-fail",
@ -767,11 +722,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
"config directory following xdg spec on linux platform",
"",
);
opts.optflag(
"",
"clean-shutdown",
"Do not shutdown until all threads have finished (macos only)",
);
opts.optflag("v", "version", "Display servo version information");
opts.optflag("", "unminify-js", "Unminify Javascript");
opts.optopt("", "profiler-db-user", "Profiler database user", "");
@ -793,7 +743,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
// some dummy options for now.
if let Some(content_process) = opt_match.opt_str("content-process") {
MULTIPROCESS.store(true, Ordering::SeqCst);
return ArgumentParsingResult::ContentProcess(content_process);
return ArgumentParsingResult::ContentProcess(opt_match, content_process);
}
let mut debug_options = DebugOptions::default();
@ -836,15 +786,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
None => 512,
};
let device_pixels_per_px = opt_match.opt_str("device-pixel-ratio").map(|dppx_str| {
dppx_str.parse().unwrap_or_else(|err| {
args_fail(&format!(
"Error parsing option: --device-pixel-ratio ({})",
err
))
})
});
// If only the flag is present, default to a 5 second period for both profilers
let time_profiling = if opt_match.opt_present("p") {
match opt_match.opt_str("p") {
@ -986,9 +927,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
})
.collect();
let do_not_use_native_titlebar =
opt_match.opt_present("b") || !(pref!(shell.native_titlebar.enabled));
let enable_subpixel_text_antialiasing =
!debug_options.disable_subpixel_aa && pref!(gfx.subpixel_text_antialiasing.enabled);
@ -998,7 +936,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
is_running_problem_test: is_running_problem_test,
url: url_opt,
tile_size: tile_size,
device_pixels_per_px: device_pixels_per_px,
time_profiling: time_profiling,
time_profiler_trace_path: opt_match.opt_str("profiler-trace-path"),
mem_profiler_period: mem_profiler_period,
@ -1010,7 +947,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
gc_profile: debug_options.gc_profile,
load_webfonts_synchronously: debug_options.load_webfonts_synchronously,
headless: opt_match.opt_present("z"),
angle: opt_match.opt_present("angle"),
hard_fail: opt_match.opt_present("f") && !opt_match.opt_present("F"),
bubble_inline_sizes_separately: bubble_inline_sizes_separately,
profile_script_events: debug_options.profile_script_events,
@ -1040,10 +976,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
style_sharing_stats: debug_options.style_sharing_stats,
convert_mouse_to_touch: debug_options.convert_mouse_to_touch,
exit_after_load: opt_match.opt_present("x"),
no_native_titlebar: do_not_use_native_titlebar,
enable_vsync: !debug_options.disable_vsync,
webrender_stats: debug_options.webrender_stats,
use_msaa: debug_options.use_msaa,
config_dir: opt_match.opt_str("config-dir").map(Into::into),
full_backtraces: debug_options.full_backtraces,
is_printing_version: is_printing_version,
@ -1055,7 +988,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
certificate_path: opt_match.opt_str("certificate-path"),
unminify_js: opt_match.opt_present("unminify-js"),
print_pwm: opt_match.opt_present("print-pwm"),
clean_shutdown: opt_match.opt_present("clean-shutdown"),
};
set_options(opts);
@ -1074,12 +1006,12 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
set_pref!(layout.threads, layout_threads as i64);
}
ArgumentParsingResult::ChromeProcess
return ArgumentParsingResult::ChromeProcess(opt_match);
}
pub enum ArgumentParsingResult {
ChromeProcess,
ContentProcess(String),
ChromeProcess(Matches),
ContentProcess(Matches, String),
}
// Make Opts available globally. This saves having to clone and pass

View file

@ -464,6 +464,10 @@ pub struct Constellation<Message, LTF, STF> {
/// Mechanism to force the compositor to process events.
event_loop_waker: Option<Box<dyn EventLoopWaker>>,
/// The ratio of device pixels per px at the default scale. If unspecified, will use the
/// platform default setting.
device_pixels_per_px: Option<f32>,
}
/// State needed to construct a constellation.
@ -520,6 +524,10 @@ pub struct InitialConstellationState {
/// Mechanism to force the compositor to process events.
pub event_loop_waker: Option<Box<dyn EventLoopWaker>>,
/// The ratio of device pixels per px at the default scale. If unspecified, will use the
/// platform default setting.
pub device_pixels_per_px: Option<f32>,
}
/// Data needed for webdriver
@ -837,6 +845,7 @@ where
glplayer_threads: state.glplayer_threads,
player_context: state.player_context,
event_loop_waker: state.event_loop_waker,
device_pixels_per_px,
};
constellation.run();
@ -1081,6 +1090,7 @@ where
webxr_registry: self.webxr_registry.clone(),
player_context: self.player_context.clone(),
event_loop_waker: self.event_loop_waker.as_ref().map(|w| (*w).clone_box()),
device_pixels_per_px: self.device_pixels_per_px,
});
let pipeline = match result {

View file

@ -205,6 +205,10 @@ pub struct InitialPipelineState {
/// Mechanism to force the compositor to process events.
pub event_loop_waker: Option<Box<dyn EventLoopWaker>>,
/// The ratio of device pixels per px at the default scale. If unspecified, will use the
/// platform default setting.
pub device_pixels_per_px: Option<f32>,
}
pub struct NewPipeline {
@ -312,6 +316,7 @@ impl Pipeline {
webvr_chan: state.webvr_chan,
webxr_registry: state.webxr_registry,
player_context: state.player_context,
device_pixels_per_px: state.device_pixels_per_px,
};
// Spawn the child process.
@ -518,6 +523,7 @@ pub struct UnprivilegedPipelineContent {
webvr_chan: Option<IpcSender<WebVRMsg>>,
webxr_registry: webxr_api::Registry,
player_context: WindowGLContext,
device_pixels_per_px: Option<f32>,
}
impl UnprivilegedPipelineContent {
@ -609,7 +615,7 @@ impl UnprivilegedPipelineContent {
layout_thread_busy_flag.clone(),
self.opts.load_webfonts_synchronously,
self.opts.initial_window_size,
self.opts.device_pixels_per_px,
self.device_pixels_per_px,
self.opts.dump_display_list,
self.opts.dump_display_list_json,
self.opts.dump_style_tree,

View file

@ -306,7 +306,11 @@ impl<Window> Servo<Window>
where
Window: WindowMethods + 'static + ?Sized,
{
pub fn new(mut embedder: Box<dyn EmbedderMethods>, window: Rc<Window>) -> Servo<Window> {
pub fn new(
mut embedder: Box<dyn EmbedderMethods>,
window: Rc<Window>,
device_pixels_per_px: Option<f32>,
) -> Servo<Window> {
// Global configuration options, parsed from the command line.
let opts = opts::get();
@ -551,6 +555,7 @@ where
webvr_constellation_sender,
glplayer_threads,
event_loop_waker,
device_pixels_per_px,
);
// Send the constellation's swmanager sender to service worker manager thread
@ -582,7 +587,7 @@ where
opts.is_running_problem_test,
opts.exit_after_load,
opts.convert_mouse_to_touch,
opts.device_pixels_per_px,
device_pixels_per_px,
);
Servo {
@ -870,6 +875,7 @@ fn create_constellation(
webvr_constellation_sender: Option<Sender<Sender<ConstellationMsg>>>,
glplayer_threads: Option<GLPlayerThreads>,
event_loop_waker: Option<Box<dyn EventLoopWaker>>,
device_pixels_per_px: Option<f32>,
) -> (Sender<ConstellationMsg>, SWManagerSenders) {
// Global configuration options, parsed from the command line.
let opts = opts::get();
@ -912,6 +918,7 @@ fn create_constellation(
glplayer_threads,
player_context,
event_loop_waker,
device_pixels_per_px,
};
let (constellation_chan, from_swmanager_sender) = Constellation::<
script_layout_interface::message::Msg,
@ -920,7 +927,7 @@ fn create_constellation(
>::start(
initial_state,
opts.initial_window_size,
opts.device_pixels_per_px,
device_pixels_per_px,
opts.random_pipeline_closure_probability,
opts.random_pipeline_closure_seed,
opts.is_running_problem_test,