ContextualSearchLayer::SetProperties takes 51 parameters |
|||||
Issue description
In chrome/browser/android/compositor/layer/contextual_search_layer.cc, we have:
void ContextualSearchLayer::SetProperties(int panel_shadow_resource_id, int search_context_resource_id, int search_term_resource_id, int search_caption_resource_id, int search_bar_shadow_resource_id, int panel_icon_resource_id, int search_provider_icon_sprite_metadata_resource_id, int arrow_up_resource_id, int close_icon_resource_id, int progress_bar_background_resource_id, int progress_bar_resource_id, int search_promo_resource_id, int peek_promo_ripple_resource_id, int peek_promo_text_resource_id, float dp_to_px, const scoped_refptr<cc::Layer>& content_layer, bool search_promo_visible, float search_promo_height, float search_promo_opacity, bool search_peek_promo_visible, float search_peek_promo_height, float search_peek_promo_padding, float search_peek_promo_ripple_width, float search_peek_promo_ripple_opacity, float search_peek_promo_text_opacity, float search_panel_x, float search_panel_y, float search_panel_width, float search_panel_height, float search_bar_margin_side, float search_bar_height, float search_context_opacity, float search_term_opacity, float search_caption_animation_percentage, bool search_caption_visible, bool search_bar_border_visible, float search_bar_border_height, bool search_bar_shadow_visible, float search_bar_shadow_opacity, bool search_provider_icon_sprite_visible, float search_provider_icon_sprite_completion_percentage, bool thumbnail_visible, float thumbnail_visibility_percentage, int thumbnail_size, float arrow_icon_opacity, float arrow_icon_rotation, float close_icon_opacity, bool progress_bar_visible, float progress_bar_height, float progress_bar_opacity, int progress_bar_completion) {
...
}
:-\
,
Oct 4 2016
ContextualSearchSceneLayer::UpdateContextualSearchLayer - 58 params. I'm going to stop looking now.
,
Oct 5 2016
,
Oct 5 2016
Rendering things in C++ that use parameters derived in Java leads to these long paran lists.
,
Oct 6 2016
Having a setter for each of those would require a lot of JNI calls, which would hurt performance as we do this every frame (and for tab layer for every visible tab in the switcher every frame). I don't have the performance numbers, but when the original author wrote it I believe they measured a noticeable impact (years ago). This happened because we moved all of our custom non-Android UI to the Chromium Compositor instead of doing GL calls in Java, but we still have a lot of our logic building out all of the element positions, gesture tracking, etc. in Java. We could move this to native as well which would avoid the jump, but that's a ton of work.
,
Oct 9 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 12 2017
donnd@, is this still something we're interested in fixing? |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by thestig@chromium.org
, Oct 4 2016