Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Viewer] Font size slider #12588

Merged
merged 10 commits into from Feb 5, 2024
Merged

Conversation

Tamir91
Copy link
Contributor

@Tamir91 Tamir91 commented Jan 18, 2024

This PR adds an accessibility option to the RS-Viewer + Depth Quality tool settings.

Now the user can control and increase the default application font size.

image

Tracked on [LRS-979]

@Tamir91 Tamir91 changed the title []Font size slider [Viewer] Font size slider Jan 18, 2024
@Tamir91 Tamir91 requested a review from Nir-Az January 23, 2024 13:31
common/device-model.cpp Outdated Show resolved Hide resolved
@@ -239,7 +240,7 @@ bool option_model::draw_combobox( notifications_model & model,

ImGui::SameLine();
if( new_line )
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new_line is true by default. Can we remove a check?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a parameter, is it always true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it receives true in the parameters of a function.
We call the function only once without sending it a boolean parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's always try on DQT + RS-Viewer then we can remove it sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But on another side in the future, if somebody adds a new call with draw(..., ..., false) it will create unexpected behavior. Sorry, let's leave it as is it now.


std::vector<const char*> methods;
methods.push_back("Viewer Processing Rate");
methods.push_back("Camera Timestamp Rate");

ImGui::PushItemWidth(r.w - 207);
ImGui::PushItemWidth(-1.f);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does a width of -1 do?
Looks like the older calculation is related to windows size, don't we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 value set width "to the end available zone"
ImGui::Text("SetNextItemWidth/PushItemWidth(-1)"); ImGui::SameLine(); HelpMarker("Align to right edge"); ImGui::PushItemWidth(-1);
The code sample from imgui_demo.cpp here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do the same with all combo boxes in the left pane and adjust the border according to the font size?

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jan 25, 2024

Can we align this 2 sliders to look the same?
same position, same blue line width?
image

@Tamir91
Copy link
Contributor Author

Tamir91 commented Jan 25, 2024

Can we align this 2 sliders to look the same? same position, same blue line width? image

Improved to:
image
According to blue line width: The lines are different because the first slider has 8 steps (1-8) but the second slider has 5 steps (16-20). So I must create the same ranges for 2 sliders to create a similar blue line width.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jan 25, 2024

Can we align this 2 sliders to look the same? same position, same blue line width? image

Improved to: image According to blue line width: The lines are different because the first slider has 8 steps (1-8) but the second slider has 5 steps (16-20). So I must create the same ranges for 2 sliders to create a similar blue line width.

OK, looks good thanks

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jan 25, 2024

Font 19:
Add source button
image

@Tamir91 Tamir91 requested a review from Nir-Az January 29, 2024 12:38
@Tamir91
Copy link
Contributor Author

Tamir91 commented Jan 30, 2024

Font 19: Add source button image

image
This is a screenshot with 2 cameras. It works well with 4 lines of text too.

@Tamir91 Tamir91 requested a review from Nir-Az January 30, 2024 07:10
@Tamir91
Copy link
Contributor Author

Tamir91 commented Jan 30, 2024

Font 16:
image

Font 20:
image

@Nir-Az
Copy link
Collaborator

Nir-Az commented Feb 4, 2024

Please rebase before we merge it

The combo box starts right after his label now and not after the fixed number.
The width of Combo is dynamic according to font size now.
A big_button from the tools panel (3D view) is upgraded to receive float values because the sizes of buttons must be float values due to compatibility with font size.
The Font slider was aligned with the Font Samples
@Tamir91
Copy link
Contributor Author

Tamir91 commented Feb 5, 2024

Please rebase before we merge it

Rebased

@Tamir91 Tamir91 requested a review from Nir-Az February 5, 2024 09:42
@Nir-Az Nir-Az merged commit 6926aff into IntelRealSense:development Feb 5, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants