Cutter: Add graph bitmap export options

[ps1337]

Two options would be nice for the graph bitmap export:

• Allow users to provide a scaling factor >= 100% that increases the size of the exported image
• Add an option that allows using a transparent background instead of the UI background color

I'll try to come up with a PR as soon as i have some time for it

[NirmalManoj]

Can I take up this issue? Asking because I can't see any PRs in the past 1 month.

[ps1337]

Sure, I'm quite busy atm unfortunately so if you want to give it a go, please do :)

-----Original Message-----
From: NIRMAL MANOJ C [e-mail]
To: radareorg/cutter [e-mail]
Cc: ps [e-mail], Author [e-mail]
Sent: Sat, 29 Feb 2020 18:56
Subject: Re: [radareorg/cutter] Add graph bitmap export options (#1998)

Can I take up this issue? Asking because I can't see any PRs in the past 1 month.

--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/radareorg/cutter/issues/1998#issuecomment-592974207

[ITAYC0HEN]

Can I take up this issue? Asking because I can't see any PRs in the past 1 month.

Absolutely :)

[NirmalManoj]

I am confirming that I will start working on this issue. Can @ps1337 assign this issue to me?

[NirmalManoj]

I was trying to understand this codebase playing around making minor changes. Now I am going to write a PR. There is one modification that I will be making and I doubt if it would affect any other functionality of the program.

In _GraphView::saveAsBitmap(QString path, const char *format)_ in _widgets/GraphView.cpp_, QImage is given the format of '_Format_RGB32_', however from what I have understood from the documentation and various articles, this format does not support transparency. So can I use _Format_ARGB32_ instead?
The line 403 (current)
QImage image(width, height, QImage::Format_RGB32);
will be changed to
QImage image(width, height, QImage::Format_ARGB32);

[NirmalManoj]

@karliss I couldn't find any problem caused by making this particular change. But making sure. There are some other modifications also which I think would not matter much like adding two more parameters to the _saveAsBitmap_ function.

[karliss]

@NirmalManoj So far sounds reasonable.

[NirmalManoj]

@karliss I have finished the parts for adding scaling factor and transparency as arguments to the function. Currently I am thinking about various options to make the dialog.
Can I do the following, or do you want me to make in any particular way?

1. Once user clicks save after selecting the extension as png/jpeg(bitmap), a popup dialog comes.
2. User can select transparency (checkbox) and scaling factor (QDoubleSpinBox) .
3. Then graph gets saved as per the options user selected.

I am actually confused whether I should make a seperate class to make a custom dialog like this or should I try adding this into the _MultipleFileSaveDialog_ that is being called when the Export Graph is clicked.

Edit:By MultiplyFileSaveDialog, I mean adding a transparent png/jpg to the list. But this will look very weird and I haven't even tried thinking more along this idea.

I just want to know if making a popup dialog every time the user tries to save a graph bitmap is okay.

[karliss]

The popup isn't great but I don't see a better option. It could be shown only for formats that support at least one of the options. One more potential improvement is having a don't show this again checkbox which saves in the default graph export options. It would require adding in settings a section with default export options and checkbox selecting if the popup dialog should be shown or the default settings used.

Having customized options in the list of formats would be weird.

Third option would be extending the file save dialog by adding the checkbox and spinbox to it. I prefer use of native file save and open dialogs as much as possible. Native file dialogs provided provided more consistent experience by having the same shortcuts and other navigation controls as rest of the programs, displays bookmarked folders and any other desktop specific functionality. Unfortunately we currently can't use native file save dialog due to the way format selection works. That seems to be at least Qt limitation by not exposing all the functionality provided by the native dialogs and not the limitation of native file dialogs themselves.

[NirmalManoj]

@karliss I have finished this task almost completely. Following points explain what I did.

Functionalities added:

• Edit -> Preferences -> Disassembly-Graph, user can choose if they want the exported bitmap image to be transparent or not. [Checkbox]
  At the same location, there is Double SpinBox using which user can select the scaling factor (>=100% to 30000%). The maximum being 30000% is pure arbitrary choice that I made as a factor even as large as 30000% is not useful for any situations I could think of. This can be increased if needed, but I don't think there is a point.
• Normal export, if the user selects the extensions png/jpg (bitmap), this will export with the given scaling factor. If the option transaparent is selected, png export will be transparent. Note : jpg doesn't support transparency So if a user select transparent for a jpg export, by default the background will be black.
• In a session, if a user selects the scaling factor and transparency for the exported bitmap images once, it will remain so as long they are in the current session. User can change this at any point if they want.

Changes made to code and additions.

• [ Doubt if this is against the standard followed in cutter ] : Defined two variables _double bitmapGraphExportScale_ and _bool bitmapGraphExportTransparency_ in _common/Configuration.h_ using the extern keyword and initialized this in _common/Configuration.cpp_.

These variables store the details about how much graph should be scaled while exporting and if it should be transparent or not.

• Made _getter_ and _setter_ functions for _bitmapGraphExportScale_ and _bitmapGraphExportTransparency_ in Configuration.cpp.

• In _dialogs/preferences/GraphOptionsWidget_ made a checkbox for transparency and double spinbox for scaling factor.

• When changes are made to the transparency and/or spinbox, call the respective setter function in _Configuration.cpp_ to update the value of transparency and/or spinbox.

• In _src/widgets/Graphview.cpp,_ changed the function definition of _saveAsBitmap_ from
  void void saveAsBitmap(QString path, const char *format = nullptr)
  to
  void saveAsBitmap(QString path, const char *format = nullptr, double scaler = 1.0, bool transparent = false);
  The file saved by the _saveAsBitmap_ functions is according to the scaling factor and transparent [or not] option selected by the user.

• In _src/widgets/DisassemblerGraphView.cpp,_ in function _DisassemblerGraphView::exportGraph_ two variables are assigned the current value of both _bitmapGraphExportScale_ and _bitmapGraphExportTransparency_. These variables are passed arguments if the graph is exported in bitmap format.

Changes and additions I doubt if I should make

1. Making the background for the transparent export as jpg white instead of black.
2. Making the popup dialog to choose scaling factor and transparency after clicking save [ in bitmap format ] everytime, along with don't repeat this dialog option.

[karliss]

There is no need to have such detailed description of code changes in Issue. You could have created a PR so that I can look at the code directly. If there are some minor changes left you can create a PR in draft state thus making it clear that isn't finished. The "Functionality added" part could have been Detailed description part of PR .

Doubt if this is against the standard followed in cutter

It's hard to tell without seeing code, better create a draft PR. Take a look at how other non-r2 values are stored in Configuration.cpp.

Making the background for the transparent export as jpg white instead of black.

Instead of white or black you could ignore the transparency option when not supported by file format thus resulting in background that would have been used if transparency option was disabled.

Making the popup dialog

Just having it in settings might be sufficient. It's not something you would change often.

[NirmalManoj]

@karliss
Will definitely create a PR for future issues that I will work on rather than making such a detailed description of code changes in Issue. Thanks for telling me this,

Instead of white or black you could ignore the transparency option

This was also an option, I will change code for this and then create a PR.

[NirmalManoj]

Forgot to mention here, I had made a PR https://github.com/radareorg/cutter/pull/2089 a few hours ago.