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

Enabled 2 mesh viewers #26181

Open
wants to merge 79 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

classicrocker883
Copy link
Contributor

@classicrocker883 classicrocker883 commented Aug 17, 2023

Description

for ProUI:

This adds the option to enable two types of mesh viewers.
Toggle between the older "Red ..0.. Green" square block style and the newer rainbow color round style.

  • add menu option to view 2 ways to view mesh.
  • renamed to MSG_CHANGE_MESH from MSG_CHANGE_MESH_VIEWER

changed in Conditionals_post.h so USE_GRID_MESHVIEWER is defined when ProUI is used, because bedlevelTools is.
in settings.cpp, also added a couple relevant comments to other things in settings.cpp.

Requirements

DWIN_LCD_PROUI
HAS_MESH

Benefits

gives the user the option to toggle between the two ways to view the mesh.

Configurations

Related Issues

Note

in meshviewer.cpp I wrote a TODO: comment because set_status_and_level was changed in marlinui.h (which is now in the other PR).

I'm not sure which is right... either leaving how this function is defined, and just changing how it is used in meshviewer.cpp (by adding , 0) to the end of it OR level=0 as its defined).

-   ui.set_status_and_level(MString<30>(F("Mesh Z min: "), p_float_t(min, 2), F(", max: "), p_float_t(max, 2)));
// with change in marlin.h -- level=0
+   ui.set_status_and_level(MString<30>(F("Mesh Z min: "), p_float_t(min, 2), F(", max: "), p_float_t(max, 2)), 0);
// without changes 

as example

-  static void set_status_and_level(const char * const cstr, const int8_t level) { _set_status_and_level(cstr, level, false); }
+  static void set_status_and_level(const char * const cstr, const int8_t level=0) { _set_status_and_level(cstr, level, false); }

@classicrocker883
Copy link
Contributor Author

@thisiskeithb @thinkyhead Prusa/MK3S-BigTreeTech-BTT002 has caused an issue with pull requests

(BIGTREE_BTT002) (pull_request) fails and with the given error:

Marlin\src\sd\cardreader.cpp:996:28: error: 'IS_DIR' was not declared in this scope; did you mean 'EISDIR'?
Marlin\src\sd\cardreader.cpp:1014:30: error: 'IS_DIR' was not declared in this scope; did you mean 'EISDIR'?
Marlin\src\sd\cardreader.cpp:1365:11: error: 'isDir' was not declared in this scope; did you mean 'isDigit'?

HAS_FOLDER_SORTING should be enabled, because FOLDER_SORTING became undefined in the recent push to example configs.

@thisiskeithb
Copy link
Member

Thinkyhead will need to push a follow-up commit after checking existing configs that were broken after MarlinFirmware/Configurations@65c5120.

@thinkyhead
Copy link
Member

The viewer_asymmetric_range flag is still referenced, so that cannot be removed just yet. If it is no longer relevant then the code that refers to it must be updated. Check with @mriscoc for advice on whether it should be retained.

BACK_ITEM(drawPrepareMenu);
MENU_ITEM(ICON_ManualMesh, MSG_LEVEL_BED, onDrawMenuItem, manualMeshStart);
mMeshMoveZItem = EDIT_ITEM(ICON_Zoffset, MSG_MOVE_Z, onDrawMMeshMoveZ, setMMeshMoveZ, &current_position.z);
MENU_ITEM(ICON_Axis, MSG_UBL_CONTINUE_MESH, onDrawMenuItem, manualMeshContinue);
MENU_ITEM(ICON_MeshViewer, MSG_MESH_VIEW, onDrawSubMenu, dwinMeshViewer);
#if USE_GRID_MESHVIEWER
EDIT_ITEM(ICON_PrintSize, MSG_CHANGE_MESH_VIEWER, onDrawChkbMenu, setViewMesh, &bedLevelTools.view_mesh);
Copy link
Member

Choose a reason for hiding this comment

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

In what way does this "change" the mesh viewer?

Copy link
Contributor Author

@classicrocker883 classicrocker883 Aug 19, 2023

Choose a reason for hiding this comment

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

there are two types of ways to view the mesh

This shows the two types of meshviewer as an example

you can toggle between with the checkbox

Copy link
Member

Choose a reason for hiding this comment

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

The display with solid colors is much nicer. The display with circles seems noisy and inferior, but is it useful for people with color blindness?

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, you get my point. The label "Change Mesh Viewer" with a checkbox next to it is unclear and therefore bad UI design. The label should indicate what type of mesh viewer will be seen when the checkbox is checked. For example, the label "Ugly Mesh View" would make it clear that when the checkbox is checked the mesh viewer will be the "ugly" version, as opposed to the "normal" version.

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 display with solid colors is much nicer. The display with circles seems noisy and inferior, but is it useful for people with color blindness?

That's a valid point. I'm not partial to either, just offering making the options more available. I just wanted to say the picture examples posted aren't reminiscent of a true bed level, merely did a quick manual edit of the values to give a quick example of how it can be displayed.

this above image is more a true representation of a typical mesh.

anyway it is what it is. and as for calling it "Normal Mesh View" or something like that is a good idea. Having the checkbox enabled means to have the Square block style. default would be the box unchecked and Circle colorful style.

@mriscoc
Copy link
Contributor

mriscoc commented Aug 18, 2023

The viewer_asymmetric_range flag is still referenced, so that cannot be removed just yet. If it is no longer relevant then the code that refers to it must be updated. Check with @mriscoc for advice on whether it should be retained.

I didn't see what was wrong with the thumbnail preview. Proper definitions should be ensured to allow people to choose between having a normal mesh viewer, a grid mesh viewer, or a selectable one; otherwise, we're increasing the program memory usage for no real benefit.

@classicrocker883
Copy link
Contributor Author

classicrocker883 commented Aug 19, 2023

I didn't see what was wrong with the thumbnail preview.

it wasnt working when I tried before I made the changes. unless I imagined it. but it was working fine after.

update: not sure what happened. thumbnail previews are working without changes made. so disregard.

@classicrocker883
Copy link
Contributor Author

I made some changes to this PR which should be easier to merge now.
take a look at the Note in description for a small concern/issue

@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 2 times, most recently from 9c65146 to 4f65466 Compare January 26, 2024 00:13
@classicrocker883
Copy link
Contributor Author

@thinkyhead I believe we can merge this now if you dont mind taking a quick review

@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from 37d77d6 to aa44542 Compare September 28, 2024 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants