UI: Search combo property item with QVariant type by norihiro · Pull Request #6788 · obsproject/obs-studio

norihiro · 2022-07-23T02:18:37Z

Description

When finding current item for a combobox on properties view, use QVariant type to hold current values instead of converting all types into string.

Motivation and Context

A combobox created with OBS_COMBO_FORMAT_FLOAT cannot show the current item. When a modified callback for another item on the properties view returns true, the combobox is initialized again and the selection goes to the top item even though another item is selected.

When adding items into the combobox, userData is generated in the code below.

obs-studio/UI/properties-view.cpp

Lines 505 to 523 in e3f416f

    
           static void AddComboItem(QComboBox *combo, obs_property_t *prop, 
        
           			 obs_combo_format format, size_t idx) 
        
           { 
        
           	const char *name = obs_property_list_item_name(prop, idx); 
        
           	QVariant var; 
        
           	if (format == OBS_COMBO_FORMAT_INT) { 
        
           		long long val = obs_property_list_item_int(prop, idx); 
        
           		var = QVariant::fromValue<long long>(val); 
        
           	} else if (format == OBS_COMBO_FORMAT_FLOAT) { 
        
           		double val = obs_property_list_item_float(prop, idx); 
        
           		var = QVariant::fromValue<double>(val); 
        
           	} else if (format == OBS_COMBO_FORMAT_STRING) { 
        
           		var = QByteArray(obs_property_list_item_string(prop, idx)); 
        
           	} 
        
           	combo->addItem(QT_UTF8(name), var);

OBS's type	`userData` generation function
`OBS_COMBO_FORMAT_INT`	`QVariant::fromValue<long long>(long long)`
`OBS_COMBO_FORMAT_FLOAT`	`QVariant::fromValue<double>(double)`
`OBS_COMBO_FORMAT_STRING`	`QByteArray(const char *)`

I'm implementing a plugin on norihiro/obs-color-monitor#54 and found this bug.

How Has This Been Tested?

OS: Fedora 35
Qt version: 5.15.2-31.fc35 and 6.2.3-2.fc35 (I checked with both versions.)

Checked these steps for xshm_input (using OBS_COMBO_FORMAT_INT) and slideshow (using OBS_COMBO_FORMAT_STRING).

Create a source.
Open it's properties view.
Choose another item in combobox.
Close the properties view and open it again.
Check the combobox shows the same item.
- Before this bugfix, the top item is always selected at this step for the combobox with OBS_COMBO_FORMAT_FLOAT.

Also checked another source under development using OBS_COMBO_FORMAT_FLOAT.

Need to check autoselect functionality but only available on two sources av_capture_input on macOS and dshow_input on Windows.

I've checked the signature and description of QComboBox::findData are same in between Qt5 and Qt6.
Qt5: https://doc.qt.io/qt-5/qcombobox.html#findData
Qt6: https://doc.qt.io/qt-6/qcombobox.html#findData

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code has been run through clang-format.
I have read the contributing document.
My code is not on the master branch.
The code has been tested.
- I didn't test autoselect functionality.
- I didn't test on Windows though especially the behavior of QT_UTF8 is different on Windows.
All commit messages are properly formatted and commits squashed where appropriate.
I have included updates to all appropriate documentation.

RytoEX · 2022-07-23T02:29:14Z

I've checked the signature and description of QComboBox::findData are same in between Qt5 and Qt6.
Qt5: https://doc.qt.io/qt-5/qcombobox.html#findData
Qt6: https://doc.qt.io/qt-6/qcombobox.html#findData

See QTBUG-104990.
cc: @gxalpha

norihiro · 2022-07-23T02:51:23Z

However QTBUG-104990 claims the bug was introduced in Qt6, the issue was found in Qt5. Maybe OBS_COMBO_FORMAT_INT is broken by Qt6 but OBS_COMBO_FORMAT_FLOAT is not working in both Qt5 and Qt6.

RytoEX · 2022-07-23T03:07:37Z

However QTBUG-104990 claims the bug was introduced in Qt6, the issue was found in Qt5. Maybe OBS_COMBO_FORMAT_INT is broken by Qt6 but OBS_COMBO_FORMAT_FLOAT is not working in both Qt5 and Qt6.

I'm saying that the behavior of QComboBox::findData changes between Qt5 and Qt6, which is why I highlighted the text saying that the function is the same between Qt5 and Qt6.

Bring the modification in obsproject/obs-studio#6788

jpark37 · 2022-07-28T05:42:10Z

I know I just added two (optional) defects, but please merge this sooner than later. It's quite a nasty bug. The code looks good otherwise.

jpark37

Update the commit message. This is no longer just about OBS_COMBO_FORMAT_FLOAT.

A combobox with `OBS_COMBO_FORMAT_FLOAT` type couldn't be initialized to select current item from the settings data. This commit will create QVariant value with the similar code as that of adding the items and search the current item using that value.

jpark37 · 2022-07-28T09:15:23Z

Looks good to me. Someone merge this ASAP as possible.

sebastian-s-beckmann

Seems to work fine, code changes make sense to me. In an ideal world, I'd love to hear back from QTBUG-104990 first, but even if it's a bug it would exist for some time so we'd need this anyways, so we should probably just merge it.

RytoEX added the kind/bug Categorizes issue or PR as related to a bug. label Jul 23, 2022

norihiro added a commit to norihiro/obs-color-monitor that referenced this pull request Jul 23, 2022

Fix current item on float-type combobox property

a57ce1f

Bring the modification in obsproject/obs-studio#6788

norihiro added a commit to norihiro/obs-color-monitor that referenced this pull request Jul 25, 2022

Fix current item on float-type combobox property

4277e12

Bring the modification in obsproject/obs-studio#6788

norihiro added a commit to norihiro/obs-color-monitor that referenced this pull request Jul 25, 2022

Fix current item on float-type combobox property

874d3a2

Bring the modification in obsproject/obs-studio#6788

norihiro added a commit to norihiro/obs-color-monitor that referenced this pull request Jul 25, 2022

Fix current item on float-type combobox property

1e1b4b2

Bring the modification in obsproject/obs-studio#6788

norihiro added a commit to norihiro/obs-color-monitor that referenced this pull request Jul 25, 2022

Fix current item on float-type combobox property

978fde1

Bring the modification in obsproject/obs-studio#6788

jpark37 reviewed Jul 28, 2022

View reviewed changes

Comment thread UI/properties-view.cpp Outdated

jpark37 reviewed Jul 28, 2022

View reviewed changes

Comment thread UI/properties-view.cpp Outdated

jpark37 suggested changes Jul 28, 2022

View reviewed changes

jpark37 added this to the OBS Studio 28.0 milestone Jul 28, 2022

norihiro force-pushed the ui-combo-float branch from c400a1d to 0051460 Compare July 28, 2022 09:13

jpark37 approved these changes Jul 28, 2022

View reviewed changes

norihiro force-pushed the ui-combo-float branch from 0051460 to f886df4 Compare July 28, 2022 09:15

norihiro changed the title ~~UI: Fix current item on float-type combobox property~~ UI: Search combo property item with QVariant type Jul 28, 2022

sebastian-s-beckmann approved these changes Jul 28, 2022

View reviewed changes

RytoEX merged commit 766d1bb into obsproject:master Jul 28, 2022

norihiro deleted the ui-combo-float branch July 29, 2022 00:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

UI: Search combo property item with QVariant type#6788

UI: Search combo property item with QVariant type#6788
RytoEX merged 1 commit into
obsproject:masterfrom
norihiro:ui-combo-float

norihiro commented Jul 23, 2022 •

edited

Loading

Uh oh!

RytoEX commented Jul 23, 2022

Uh oh!

norihiro commented Jul 23, 2022

Uh oh!

RytoEX commented Jul 23, 2022

Uh oh!

Uh oh!

Uh oh!

jpark37 commented Jul 28, 2022

Uh oh!

jpark37 left a comment

Uh oh!

jpark37 commented Jul 28, 2022

Uh oh!

sebastian-s-beckmann left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	static void AddComboItem(QComboBox combo, obs_property_t prop,
	obs_combo_format format, size_t idx)
	{
	const char *name = obs_property_list_item_name(prop, idx);
	QVariant var;

	if (format == OBS_COMBO_FORMAT_INT) {
	long long val = obs_property_list_item_int(prop, idx);
	var = QVariant::fromValue<long long>(val);

	} else if (format == OBS_COMBO_FORMAT_FLOAT) {
	double val = obs_property_list_item_float(prop, idx);
	var = QVariant::fromValue<double>(val);

	} else if (format == OBS_COMBO_FORMAT_STRING) {
	var = QByteArray(obs_property_list_item_string(prop, idx));
	}

	combo->addItem(QT_UTF8(name), var);

Uh oh!

Uh oh!

Conversation

norihiro commented Jul 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

RytoEX commented Jul 23, 2022

Uh oh!

norihiro commented Jul 23, 2022

Uh oh!

RytoEX commented Jul 23, 2022

Uh oh!

Uh oh!

Uh oh!

jpark37 commented Jul 28, 2022

Uh oh!

jpark37 left a comment

Choose a reason for hiding this comment

Uh oh!

jpark37 commented Jul 28, 2022

Uh oh!

sebastian-s-beckmann left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

norihiro commented Jul 23, 2022 •

edited

Loading