Skip to content

Commit

Permalink
Use wxBitmapBundle to manage bitmap scales
Browse files Browse the repository at this point in the history
Use wxBitmapBundle to make wx handle picking the correct bitmap for any
given DPI scale. Since these are slightly more expensive to construct
(since all bitmap sizes need to be parsed), cache the results.
Unfortunately, wx doesn't allow setting a wxBitmapBundle's desired scale
or DIP display size after constructing it, so we have to construct new
bitmaps for every display size (which also makes the cache less
effective).
  • Loading branch information
arch1t3cht committed Jan 30, 2025
1 parent ea0603c commit 14125f9
Show file tree
Hide file tree
Showing 31 changed files with 119 additions and 57 deletions.
4 changes: 2 additions & 2 deletions src/audio_karaoke.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ AudioKaraoke::AudioKaraoke(wxWindow *parent, agi::Context *c)
{
using std::bind;

cancel_button = new wxBitmapButton(this, -1, GETIMAGE(kara_split_cancel_16));
cancel_button = new wxBitmapButton(this, -1, GETBUNDLE(kara_split_cancel, 16));
cancel_button->SetToolTip(_("Discard all uncommitted splits"));
cancel_button->Bind(wxEVT_BUTTON, bind(&AudioKaraoke::CancelSplit, this));

accept_button = new wxBitmapButton(this, -1, GETIMAGE(kara_split_accept_16));
accept_button = new wxBitmapButton(this, -1, GETBUNDLE(kara_split_accept, 16));
accept_button->SetToolTip(_("Commit splits"));
accept_button->Bind(wxEVT_BUTTON, bind(&AudioKaraoke::AcceptSplit, this));

Expand Down
12 changes: 4 additions & 8 deletions src/command/command.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <string>
#include <vector>

#include <wx/bitmap.h>
#include <wx/bmpbndl.h>
#include <wx/intl.h>
#include <wx/string.h>

Expand All @@ -37,12 +37,8 @@ namespace agi { struct Context; }
#define STR_HELP(a) wxString StrHelp() const override { return _(a); }
#define CMD_TYPE(a) int Type() const override { using namespace cmd; return a; }

#define CMD_ICON(icon) wxBitmap Icon(int size, double scale = 1.0, wxLayoutDirection dir = wxLayout_LeftToRight) const override { \
if (size * scale >= 64) return GETIMAGEDIR(icon##_64, scale, dir); \
if (size * scale >= 48) return GETIMAGEDIR(icon##_48, scale, dir); \
if (size * scale >= 32) return GETIMAGEDIR(icon##_32, scale, dir); \
if (size * scale >= 24) return GETIMAGEDIR(icon##_24, scale, dir); \
return GETIMAGEDIR(icon##_16, scale, dir); \
#define CMD_ICON(icon) wxBitmapBundle Icon(int height, wxLayoutDirection dir = wxLayout_LeftToRight) const override { \
return GETBUNDLEDIR(icon, height, dir); \
}

#define COMMAND_GROUP(cname, cmdname, menu, disp, help) \
Expand Down Expand Up @@ -113,7 +109,7 @@ DEFINE_EXCEPTION(CommandNotFound, CommandError);

/// Request icon.
/// @param size Icon size.
virtual wxBitmap Icon(int size, double scale = 1.0, wxLayoutDirection = wxLayout_LeftToRight) const { return wxBitmap{}; }
virtual wxBitmapBundle Icon(int height = 16, wxLayoutDirection = wxLayout_LeftToRight) const { return wxBitmapBundle{}; }

/// Command function
virtual void operator()(agi::Context *c)=0;
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_attachments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ DialogAttachments::DialogAttachments(wxWindow *parent, AssFile *ass)
: d(parent, -1, _("Attachment List"))
, ass(ass)
{
d.SetIcon(GETICON(attach_button_16));
d.SetIcons(GETICONS(attach_button));

listView = new wxListView(&d, -1, wxDefaultPosition, wxSize(500, 200));
UpdateList();
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_automation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ DialogAutomation::DialogAutomation(agi::Context *c)
, global_manager(config::global_scripts)
, global_scripts_changed(global_manager->AddScriptChangeListener(&DialogAutomation::RebuildList, this))
{
SetIcon(GETICON(automation_toolbutton_16));
SetIcons(GETICONS(automation_toolbutton));

// create main controls
list = new wxListView(this, -1, wxDefaultPosition, wxSize(600, 175), wxLC_REPORT|wxLC_SINGLE_SEL);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_autosave.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class DialogAutosave {
DialogAutosave::DialogAutosave(wxWindow *parent)
: d(parent, -1, _("Open autosave file"), wxDefaultPosition, wxSize(800, 350), wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER)
{
d.SetIcon(GETICON(open_toolbutton_16));
d.SetIcons(GETICONS(open_toolbutton));

wxSizer *files_box = new wxStaticBoxSizer(wxVERTICAL, &d, _("Files"));
file_list = new wxListBox(&d, -1);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_colorpicker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ DialogColorPicker::DialogColorPicker(wxWindow *parent, agi::Color initial_color,
preview_box = new wxStaticBitmap(this, -1, wxBitmap(40, 40, 24), wxDefaultPosition, wxSize(40, 40), STATIC_BORDER_FLAG);
recent_box = new ColorPickerRecent(this, 8, 4, 16);

eyedropper_bitmap = GETIMAGE(eyedropper_tool_24);
eyedropper_bitmap = GETBUNDLE(eyedropper_tool, 24).GetBitmapFor(this);
eyedropper_bitmap.SetMask(new wxMask(eyedropper_bitmap, wxColour(255, 0, 255)));
screen_dropper_icon = new wxStaticBitmap(this, -1, eyedropper_bitmap, wxDefaultPosition, wxDefaultSize, wxRAISED_BORDER);
screen_dropper = new ColorPickerScreenDropper(this, 7, 7, 8);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_dummy_video.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ wxComboBox *resolution_shortcuts(wxWindow *parent, int width, int height) {
DialogDummyVideo::DialogDummyVideo(wxWindow *parent)
: d(parent, -1, _("Dummy video options"))
{
d.SetIcon(GETICON(use_dummy_video_menu_16));
d.SetIcons(GETICONS(use_dummy_video_menu));

auto res_sizer = new wxBoxSizer(wxHORIZONTAL);
res_sizer->Add(spin_ctrl(&d, 1, 10000, &width), wxSizerFlags(1).Expand());
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ DialogExport::DialogExport(agi::Context *c)
, c(c)
, exporter(c)
{
d.SetIcon(GETICON(export_menu_16));
d.SetIcons(GETICONS(export_menu));
d.SetExtraStyle(wxWS_EX_VALIDATE_RECURSIVELY);

std::vector<std::string> filters = exporter.GetAllFilterNames();
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_fonts_collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ DialogFontsCollector::DialogFontsCollector(agi::Context *c)
, subs(c->ass.get())
, path(*c->path)
{
SetIcon(GETICON(font_collector_button_16));
SetIcons(GETICONS(font_collector_button));

wxString modes[] = {
_("Check fonts for availability")
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_jumpto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ DialogJumpTo::DialogJumpTo(agi::Context *c)
, c(c)
, jumpframe(c->videoController->GetFrameN())
{
d.SetIcon(GETICON(jumpto_button_16));
d.SetIcons(GETICONS(jumpto_button));

auto LabelFrame = new wxStaticText(&d, -1, _("Frame: "));
auto LabelTime = new wxStaticText(&d, -1, _("Time: "));
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_kara_timing_copy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ DialogKanjiTimer::DialogKanjiTimer(agi::Context *c)
: wxDialog(c->parent, -1, _("Kanji timing"))
, subs(c->ass.get())
{
SetIcon(GETICON(kara_timing_copier_16));
SetIcons(GETICONS(kara_timing_copier));

wxSizer *DisplayBoxSizer = new wxStaticBoxSizer(wxVERTICAL,this,_("Text"));
wxSizer *StylesBoxSizer = new wxStaticBoxSizer(wxVERTICAL,this,_("Styles"));
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_properties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ DialogProperties::DialogProperties(agi::Context *c)
: d(c->parent, -1, _("Script Properties"))
, c(c)
{
d.SetIcon(GETICON(properties_toolbutton_16));
d.SetIcons(GETICONS(properties_toolbutton));

// Button sizer
// Create buttons first. See:
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_resample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ DialogResample::DialogResample(agi::Context *c, ResampleSettings &settings)
: d(c->parent, -1, _("Resample Resolution"))
, c(c)
{
d.SetIcon(GETICON(resample_toolbutton_16));
d.SetIcons(GETICONS(resample_toolbutton));

memset(&settings, 0, sizeof(settings));
c->ass->GetResolution(script_w, script_h);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ DialogSelection::DialogSelection(agi::Context *c) :
wxDialog (c->parent, -1, _("Select"), wxDefaultPosition, wxDefaultSize, wxCAPTION)
, con(c)
{
SetIcon(GETICON(select_lines_button_16));
SetIcons(GETICONS(select_lines_button));

wxSizer *main_sizer = new wxBoxSizer(wxVERTICAL);

Expand Down
2 changes: 1 addition & 1 deletion src/dialog_shift_times.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ DialogShiftTimes::DialogShiftTimes(agi::Context *context)
, timecodes_loaded_slot(context->project->AddTimecodesListener(&DialogShiftTimes::OnTimecodesLoaded, this))
, selected_set_changed_slot(context->selectionController->AddSelectionListener(&DialogShiftTimes::OnSelectedSetChanged, this))
{
SetIcon(GETICON(shift_times_toolbutton_16));
SetIcons(GETICONS(shift_times_toolbutton));

// Create controls
shift_by_time = new wxRadioButton(this, -1, _("&Time: "), wxDefaultPosition, wxDefaultSize, wxRB_GROUP);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_spellchecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ DialogSpellChecker::DialogSpellChecker(agi::Context *context)
, context(context)
, spellchecker(SpellCheckerFactory::GetSpellChecker())
{
SetIcon(GETICON(spellcheck_toolbutton_16));
SetIcons(GETICONS(spellcheck_toolbutton));

wxSizer *main_sizer = new wxBoxSizer(wxVERTICAL);

Expand Down
2 changes: 1 addition & 1 deletion src/dialog_style_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ DialogStyleEditor::DialogStyleEditor(wxWindow *parent, AssStyle *style, agi::Con

work = std::make_unique<AssStyle>(*style);

SetIcon(GETICON(style_toolbutton_16));
SetIcons(GETICONS(style_toolbutton));

auto add_with_label = [&](wxSizer *sizer, wxString const& label, wxWindow *ctrl) {
sizer->Add(new wxStaticText(this, -1, label), wxSizerFlags().Center().Border(wxLEFT | wxRIGHT));
Expand Down
14 changes: 7 additions & 7 deletions src/dialog_style_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class DialogStyleManager final : public wxDialog {
DialogStyleManager(agi::Context *context);
};

wxBitmapButton *add_bitmap_button(wxWindow *parent, wxSizer *sizer, wxBitmap const& img, wxString const& tooltip) {
wxBitmapButton *add_bitmap_button(wxWindow *parent, wxSizer *sizer, wxBitmapBundle const& img, wxString const& tooltip) {
wxBitmapButton *btn = new wxBitmapButton(parent, -1, img);
btn->SetToolTip(tooltip);
sizer->Add(btn, wxSizerFlags().Expand());
Expand All @@ -184,11 +184,11 @@ wxSizer *make_move_buttons(wxWindow *parent, wxButton **up, wxButton **down, wxB
wxSizer *sizer = new wxBoxSizer(wxVERTICAL);
sizer->AddStretchSpacer(1);

*up = add_bitmap_button(parent, sizer, GETIMAGE(arrow_up_24), _("Move style up"));
*down = add_bitmap_button(parent, sizer, GETIMAGE(arrow_down_24), _("Move style down"));
*top = add_bitmap_button(parent, sizer, GETIMAGE(arrow_up_stop_24), _("Move style to top"));
*bottom = add_bitmap_button(parent, sizer, GETIMAGE(arrow_down_stop_24), _("Move style to bottom"));
*sort = add_bitmap_button(parent, sizer, GETIMAGE(arrow_sort_24), _("Sort styles alphabetically"));
*up = add_bitmap_button(parent, sizer, GETBUNDLE(arrow_up, 24), _("Move style up"));
*down = add_bitmap_button(parent, sizer, GETBUNDLE(arrow_down, 24), _("Move style down"));
*top = add_bitmap_button(parent, sizer, GETBUNDLE(arrow_up_stop, 24), _("Move style to top"));
*bottom = add_bitmap_button(parent, sizer, GETBUNDLE(arrow_down_stop, 24), _("Move style to bottom"));
*sort = add_bitmap_button(parent, sizer, GETBUNDLE(arrow_sort, 24), _("Sort styles alphabetically"));

sizer->AddStretchSpacer(1);
return sizer;
Expand Down Expand Up @@ -266,7 +266,7 @@ DialogStyleManager::DialogStyleManager(agi::Context *context)
}))
{
using std::bind;
SetIcon(GETICON(style_toolbutton_16));
SetIcons(GETICONS(style_toolbutton));

// Catalog
wxSizer *CatalogBox = new wxStaticBoxSizer(wxHORIZONTAL,this,_("Catalog of available storages"));
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_styling_assistant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ DialogStyling::DialogStyling(agi::Context *context)
, c(context)
, active_line_connection(context->selectionController->AddActiveLineListener(&DialogStyling::OnActiveLineChanged, this))
{
SetIcon(GETICON(styling_toolbutton_16));
SetIcons(GETICONS(styling_toolbutton));

wxSizer *main_sizer = new wxBoxSizer(wxVERTICAL);
wxSizer *bottom_sizer = new wxBoxSizer(wxHORIZONTAL);
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_timing_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ DialogTimingProcessor::DialogTimingProcessor(agi::Context *c)
{
using std::bind;

d.SetIcon(GETICON(timing_processor_toolbutton_16));
d.SetIcons(GETICONS(timing_processor_toolbutton));

// Read options
leadIn = OPT_GET("Tool/Timing Post Processor/Lead/IN")->GetInt();
Expand Down
2 changes: 1 addition & 1 deletion src/dialog_translation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ DialogTranslation::DialogTranslation(agi::Context *c)
, active_line(c->selectionController->GetActiveLine())
, line_count(c->ass->Events.size())
{
SetIcon(GETICON(translation_toolbutton_16));
SetIcons(GETICONS(translation_toolbutton));

wxSizer *main_sizer = new wxBoxSizer(wxVERTICAL);

Expand Down
6 changes: 2 additions & 4 deletions src/hotkey_data_view_model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,9 @@ class HotkeyModelCombo final : public HotkeyModelItem {
if (col == 0)
variant = to_wx(combo.Str());
else if (col == 1) {
wxIcon icon;
wxBitmapBundle icon;
try {
auto icon_bmp = cmd::get(combo.CmdName())->Icon(16);
if (icon_bmp.IsOk())
icon.CopyFromBitmap(icon_bmp);
icon = cmd::get(combo.CmdName())->Icon();
}
catch (agi::Exception const&) {
// Just use no icon; error is reported in the description column
Expand Down
51 changes: 46 additions & 5 deletions src/libresrc/libresrc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@

#include "libresrc.h"

#include <map>

#include <wx/bitmap.h>
#include <wx/bmpbndl.h>
#include <wx/icon.h>
#include <wx/iconbndl.h>
#include <wx/image.h>
#include <wx/intl.h>
#include <wx/mstream.h>

wxBitmap libresrc_getimage(const unsigned char *buff, size_t size, double scale, int dir) {
wxBitmap libresrc_getimage(const unsigned char *buff, size_t size, int dir) {
wxMemoryInputStream mem(buff, size);
if (dir != wxLayout_RightToLeft)
// Since wxWidgets 3.1.0, there is an undocumented third parameter in the ctor of wxBitmap from wxImage
// This "scale" parameter sets the logical scale factor of the created wxBitmap
return wxBitmap(wxImage(mem), wxBITMAP_SCREEN_DEPTH, scale);
return wxBitmap(wxImage(mem).Mirror(), wxBITMAP_SCREEN_DEPTH, scale);
return wxBitmap(wxImage(mem));
return wxBitmap(wxImage(mem).Mirror());
}

wxIcon libresrc_geticon(const unsigned char *buff, size_t size) {
Expand All @@ -35,3 +37,42 @@ wxIcon libresrc_geticon(const unsigned char *buff, size_t size) {
icon.CopyFromBitmap(wxBitmap(wxImage(mem)));
return icon;
}

wxBitmapBundle libresrc_getbitmapbundle(const LibresrcBlob *images, size_t count, int height, int dir) {
// This function should only ever be called on the GUI thread but declaring this thread_local is the safe way
thread_local std::map<std::tuple<const LibresrcBlob *, int, int>, wxBitmapBundle> cache;
auto key = std::make_tuple(images, height, dir);

if (auto cached = cache.find(key); cached != cache.end()) {
return cached->second;
}

wxVector<wxBitmap> bitmaps;
bitmaps.reserve(count);
for (size_t i = 0; i < count; i++) {
bitmaps.push_back(libresrc_getimage(images[i].data, images[i].size, dir));
bitmaps.back().SetScaleFactor(double(images[i].scale) / height);
}

auto bundle = wxBitmapBundle::FromBitmaps(bitmaps);
cache[key] = bundle;

return bundle;
}

wxIconBundle libresrc_geticonbundle(const LibresrcBlob *images, size_t count) {
thread_local std::map<const LibresrcBlob *, wxIconBundle> cache;

if (auto cached = cache.find(images); cached != cache.end()) {
return cached->second;
}

wxIconBundle bundle;
for (size_t i = 0; i < count; i++) {
bundle.AddIcon(libresrc_geticon(images[i].data, images[i].size));
}

cache[images] = bundle;

return bundle;
}
24 changes: 21 additions & 3 deletions src/libresrc/libresrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,36 @@
#include <cstdlib>
#include <string_view>

struct LibresrcBlob {
const unsigned char *data;
size_t size;
int scale;
};

#include "bitmap.h"
#include "default_config.h"

#include <wx/version.h>

class wxBitmap;
class wxBitmapBundle;
class wxIcon;
class wxIconBundle;

wxBitmap libresrc_getimage(const unsigned char *image, size_t size, double scale=1.0, int dir=0);
wxBitmap libresrc_getimage(const unsigned char *image, size_t size, int dir=0);
wxIcon libresrc_geticon(const unsigned char *image, size_t size);
#define GETIMAGE(a) libresrc_getimage(a, sizeof(a))
#define GETIMAGEDIR(a, s, d) libresrc_getimage(a, sizeof(a), s, d)
#define GETICON(a) libresrc_geticon(a, sizeof(a))

#define GET_DEFAULT_CONFIG(a) std::string_view(reinterpret_cast<const char *>(a), sizeof(a))

// height is the desired displayed height of the bitmap bundle in DIP.
// Having to specify this when constructing the bitmap is slightly awkward,
// but wxWidgets doesn't allow you to scale a wxBitmapBundle after creating it,
// so the scale has to be set for the individual bitmaps before constructing it.
wxBitmapBundle libresrc_getbitmapbundle(const LibresrcBlob *images, size_t count, int height, int dir=0);

wxIconBundle libresrc_geticonbundle(const LibresrcBlob *images, size_t count);

#define GETBUNDLE(a, h) libresrc_getbitmapbundle(a, sizeof(a) / sizeof(*a), h)
#define GETBUNDLEDIR(a, h, d) libresrc_getbitmapbundle(a, sizeof(a) / sizeof(*a), h, d)
#define GETICONS(a) libresrc_geticonbundle(a, sizeof(a) / sizeof(*a))
2 changes: 1 addition & 1 deletion src/menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class CommandManager {
#ifndef __WXMAC__
/// @todo Maybe make this a configuration option instead?
if (kind == wxITEM_NORMAL)
item->SetBitmap(co->Icon(16));
item->SetBitmap(co->Icon());
#endif
parent->Append(item);
items.push_back(co->name());
Expand Down
2 changes: 1 addition & 1 deletion src/preferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ void Preferences::OnResetDefault(wxCommandEvent&) {
}

Preferences::Preferences(wxWindow *parent): wxDialog(parent, -1, _("Preferences"), wxDefaultPosition, wxSize(-1, -1), wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER) {
SetIcon(GETICON(options_button_16));
SetIcons(GETICONS(options_button));

book = new wxTreebook(this, -1, wxDefaultPosition, wxDefaultSize);
General(book, this);
Expand Down
2 changes: 1 addition & 1 deletion src/subs_edit_box.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ TimeEdit *SubsEditBox::MakeTimeCtrl(wxString const& tooltip, TimeField field) {

void SubsEditBox::MakeButton(const char *cmd_name) {
cmd::Command *command = cmd::get(cmd_name);
wxBitmapButton *btn = new wxBitmapButton(this, -1, command->Icon(16, GetContentScaleFactor()));
wxBitmapButton *btn = new wxBitmapButton(this, -1, command->Icon());
ToolTipManager::Bind(btn, command->StrHelp(), "Subtitle Edit Box", cmd_name);

middle_right_sizer->Add(btn, wxSizerFlags().Expand());
Expand Down
Loading

0 comments on commit 14125f9

Please sign in to comment.