#78 - Implement Render Mode Options #89

vpstackhub · 2025-09-17T01:28:51Z

This PR completes Issue #78 by adding Render Mode options to the UI:

Users can now choose from:
- Charts (JFreeChart only)
- Table (results table only)
- Charts + Table (both shown)

Changes made:

New RenderMode enum in App.java
renderCombo dropdown added to MainFrame.java
Logic added to show/hide the chart panel depending on selected mode
UI wiring completed via Gui.java
Chart visibility tested with toggling logic

Let me know if you'd like any refinements!

vpstackhub · 2025-09-17T01:29:18Z

@jamesmarkchan ready for review – this completes #78

jamesmarkchan · 2025-09-17T04:20:10Z

@vpstackhub I was thinking this is a less likely to be configured item that the items in the main settings area so should be under the options file menu if possible and default to "Per Sample" where the graph would update per sample. If the user selected the option "Per Operation" the chart would update after a read or write operation is complete. and if a per interval is selected we would need to do some time stamping and computation if the update window has passed before the next update occurs. Should be free to tag up tomorrow or thursday or this weekend if you are free.

jamesmarkchan · 2025-09-17T04:25:28Z

i was just checking, the only two things i think we would drag over from the palette is the "Menu" and the "Menu Item". Bringing over a Menu into a Menus creates a submenu within the menu, then we'd populate it with several sub items.

Per Sample - once a sample is plotted the graph is updated.
Per Operation - once an operation is complete the graph is updated.
Per 100ms - once 100ms since the last update another update can occur.
Per 500ms - once 500ms since the last update another update can occur.
Per 1000ms - once 1s since the last update another update can occur.

jamesmarkchan · 2025-09-17T04:35:05Z

The BenchmarkWorker.java is likely where most of the action will happen.

the publish samples i believe is where the graph gets updated so if we're not on per sample, instead of just publishing the sample we'll want to put the sample into an array or some container so it can all be added later when the rendering interval is up.

jamesmarkchan · 2025-09-17T04:29:57Z

src/jdiskmark/App.java

+        TABLE,
+        CHARTS_AND_TABLE;
+
+    @Override


so here the modes would be like:

PER_SAMPLE

PER_OPERATION

PER_100MS

PER_500MS

PER_1000MS

jamesmarkchan · 2025-09-17T04:31:20Z

src/jdiskmark/App.java

+        return switch (this) {
+            case CHARTS -> "Charts";
+            case TABLE -> "Table";
+            case CHARTS_AND_TABLE -> "Charts + Table";


for your awareness lines 47-52 would be indented all by x4 spaces more.

jamesmarkchan · 2025-09-17T04:36:38Z

src/jdiskmark/App.java

+            case CHARTS_AND_TABLE -> "Charts + Table";
+            };
+        }
+    }


Maybe good to put RenderMode enum into the Gui.java class or it's own class entirely.

jamesmarkchan · 2025-09-17T04:40:44Z

it's likely we'll want to pull this from here and save the slot for a Profiles dropdown that we'll add later as part of #42.

vpstackhub · 2025-09-18T07:06:14Z

Hey James!Saw all your comments and suggestions ;totally makes sense. Thanks for the detailed breakdown.Just a heads-up: this week’s packed on my end, so I might not be able to hop on a call as soon as you proposed. That said, I wanted to make sure I understood your feedback . Here’s what I took away: • Move the RenderMode enum out of App.java, ideally into Gui.java or its own class. • Rename and redefine the enum to reflect rendering frequency, not display mode (e.g. PER_SAMPLE, PER_OPERATION, PER_100MS, etc.). • Create a new submenu under the Options menu using a JMenu and JMenuItems for the different render intervals. • Default to PER_SAMPLE, but wire in logic so other modes can be implemented later (starting with just storing the selected mode). • Possibly reserve that renderCombo slot in the future for Profiles (#42).If I missed anything major, feel free to nudge me. Otherwise I’ll try to start chipping away at this as soon as time allows.As always, thank you for your precious guidance Best,ValerioOn Sep 16, 2025, at 9:41 PM, James Mark Chan ***@***.***> wrote:jamesmarkchan left a comment (JDiskMark/jdm-java#89) it's likely we'll want to pull this from here and save the slot for a Profiles dropdown that we'll add later as part of #42. image.png (view on web) —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

jamesmarkchan · 2025-09-19T01:53:41Z

another option is we start a new dialog window called Advanced Options that is brought up from the Options Menu where the user can select a dropdown with the options we are talking about. Later we can migrate the other advanced options that are not that commonly set to this menu to keep the front primary controls for the common user clutter free and the enthusiast would bring up the advanced options to tune their benchmarks to their liking.

See #90 for a brief description of the advanced options dialogue window approach, which we can start to migrate advanced options into.

jamesmarkchan · 2025-09-24T02:06:18Z

src/jdiskmark/Gui.java

+    private static jdiskmark.RenderFrequencyMode renderFrequencyMode = jdiskmark.RenderFrequencyMode.PER_SAMPLE;
+
+    public static RenderFrequencyMode getRenderFrequencyMode() {
+    return renderFrequencyMode;


this and line 49 could be indented by 4 spaces.

jamesmarkchan · 2025-09-24T02:24:12Z

hey @vpstackhub, i checked out and tested and noticed a few things:

when selecting the render mode from the menu, the current version should be selected. There should be a way to make the Render Interval options so each option is exclusively selected (only one) and the selection is indicated. For example good to take a look at the Color Palette sub menu where only one of the palettes can be selected at a time.
the other thing is I'm not sure this is expected behavior? to see if this is a regression (breaking an existing feature) should be able to check out the original dev and see if that behavior is there. if it is than it's probably something i had introduced on dev and we should create a bug for it.
Perhaps it would be good to remove this control unless we wanted to repurpose the controls to handle the Profiles feature on the same branch?
when we test the Render mode in per operation and the Benchmark Type is Write it should not plot the graph until the entire operation is complete. Per sample behavior should be like our normal per sample update, Per 1000ms, we should see updates only every second. per 500ms updates only after half a second.
Less important but if the render mode is displayed for the benchmark, when it is selected in the Benchmark Operations tab the render mode should probably update in the graph area so we can see the render mode that was used for that benchmark. It seems to just show the render mode of what the current setting is.

Unfortunately I'm not sure what is causing some of these artifacts and how to correct at the moment but wanted to share some initial feedback.

…and MainFrame

vpstackhub · 2025-09-30T01:41:16Z

This PR completes Issue #78 by finalizing the Render Mode functionality and cleaning up the rendering logic.

Features Implemented

Render Mode Options now available in the UI:

Charts – show JFreeChart only

Table – show results table only

Charts + Table – show both

Render Mode submenu added under Options with radio buttons.

New RenderFrequencyMode enum (PER_SAMPLE, PER_OPERATION, PER_100MS, PER_500MS, PER_1000MS).

Dropdown wiring completed in MainFrame and Gui, with dynamic chart visibility toggle.

Code Changes

Consolidated duplicate logic in BenchmarkWorker:

Removed redundant switch(currentMode) blocks.

Centralized per-sample, per-operation, and interval-based buffering logic.

Added getIntervalMillis() helper for clean timing control.

Integrated with existing metrics (App.updateMetrics, publish, etc.).

Preserved thread safety and UI responsiveness with buffered sample publishing.

Testing Performed

Per Sample: results update continuously (no mid-test warnings).

Per Operation: results batch flush correctly at operation boundaries.

Per 500ms / 1000ms: results render at expected intervals, with charts/table progressively updating.

Verified toggling between Charts / Table / Charts+Table updates the UI correctly.

Clean + Build and Run behave consistently, no regressions observed.

Next Steps

Follow-up PR could focus on:

Persisting Render Mode preference in jdm.properties.

Additional GUI refinements if requested.

jamesmarkchan · 2025-09-30T19:38:58Z

Hey @vpstackhub i pulled and tested your branch really quickly but have not had a chance to look at the code, really quickly it seems there is a Render Mode in the Options menu that has different options from the Render mode on the main control panel. I feel this could confuse users and we can probably remove the Render mode in the control panel area and leave the one in the Options menu? Or were you thinking there is another render mode control we should introduce. Generally for less used controls like the Render mode it is good to hide them away in an options since it might not be of interest to most users and we can save the main control area for the primary controls. Our tool has more controls than the other competing tools. also noticed that some of the stats were not getting filled out as each record was completed.

should be free this evening if you have time for a call.

vpstackhub · 2025-10-03T19:15:13Z

Thanks @jamesmarkchan for the detailed review and testing. Based on your notes, here’s my plan:

Remove duplicate Render Mode control from the main panel → keep only under Options menu.
Update Render Interval submenu to behave like Color Palette (only one active selection at a time).
Investigate why stats in Benchmark Operations are stuck at 0/0 → confirm if regression exists in dev.
Adjust logic so:

Per Sample = update each sample
Per Operation = update only after operation completes
Per Interval = update after selected ms window passes

Ensure the Benchmark Operations tab shows the render mode used for that benchmark, not just the global setting.
Look into chart artifacts (flat zones / unexpected values).
Default render mode = Per Sample.

…rk Panel

jamesmarkchan · 2025-10-07T04:22:54Z

there are two identical pull request and I think this is the better one to keep open for this branch. it is okay to rebranch from dev and start over to reset the gui builder portions if needed.

jamesmarkchan

i think there are functional aspects to address first or possible you did not push the latest code changes to the branch. i know the generated code can be challenging to maintain, usually what i do is to revert the generated code if it gets a little too jumbled. we'll have to look at breaking up the generated code more so it is easier to maintain in the future.

jamesmarkchan · 2025-10-07T04:51:21Z

src/jdiskmark/BenchmarkPanel.java

+
+    // Dynamically add Render Mode column
+    DefaultTableModel model = (DefaultTableModel) runTable.getModel();
+    model.addColumn("Render Mode");


oh sorry i should have been more specific, even though we should save the render mode to the database i don't think we need to add a column. but we should save the value and load it and send it back to the community portal. we do need a solution to show the render mode when a selected benchmark is loaded.

jamesmarkchan · 2025-10-07T04:52:57Z

src/jdiskmark/Gui.java

+    public static void updateRenderView() {
+        RenderFrequencyMode mode = Gui.mainFrame.getRenderMode();
+
+        boolean showChart = true; // all modes show chart in this example


i'm not sure we need showChart and showTable or possible to share what you had in mind for them?

jamesmarkchan · 2025-10-07T04:54:24Z

src/jdiskmark/Gui.java

+                             mode == RenderFrequencyMode.PER_500MS ||
+                             mode == RenderFrequencyMode.PER_1000MS);
+
+        if (chartPanel != null) {


I don't believe we need to hide the chart, can likely remove lines 113-114 unless I'm overlooking something.

jamesmarkchan · 2025-10-07T04:55:37Z

src/jdiskmark/Gui.java

+            chartPanel.setVisible(showChart);
+        }
+
+        Gui.mainFrame.setTableVisible(showTable);


this is probably not needed also right? or let me know if otherwise.

jamesmarkchan · 2025-10-07T05:01:33Z

src/jdiskmark/Gui.java

+        if (renderModeLabel != null) {
+            renderModeLabel.setText("Render Mode: " + getRenderFrequencyMode());
+        }
+    }


so if a render mode is displayed on the chart i believe it should show the render mode of the active selected benchmark or the selected loaded benchmark and not the mode selected for the next benchmark because the user can see the mode selected for the next benchmark from the menu. good to keep in mind that we have controls that tell us the info of a selected or current benchmark and controls that tell us what will be used for the next benchmark that is run. I might have confused the behavior with how the main benchmark controls are being used for both. In theory it is nice to keep them separate.

jamesmarkchan · 2025-10-07T05:05:32Z

src/jdiskmark/Gui.java

+        wrapperPanel.add(chartPanel, java.awt.BorderLayout.CENTER);
+
+        return wrapperPanel;
+        }


going to ignore formatting comments for now since we'll want to get the functionality working first but normally we would not want to see the return and the closing bracket on the same column like here. the closing bracket would be lined up with the first line of the open bracket usually. does this make sense?

jamesmarkchan · 2025-10-07T05:06:47Z

src/jdiskmark/RenderFrequencyMode.java

+        return label;
+    }
+}
+


this class looks good!

jamesmarkchan · 2025-10-07T05:13:35Z

src/jdiskmark/RenderFrequencyModeAttributeConverter.java

we might not need an attribute converter class as the RenderFrequencyMode is an enumeration and we have been able to store enumerations into our derby database wo attribute converters in the past for instance there is the BenchmarkType enumeration and the IOMode enumeration. Perhaps we can model those for storing the Render frequency into the db.

jamesmarkchan · 2025-10-07T05:20:11Z

src/jdiskmark/MainFrame.java

+            orderComboBox.addItem(BenchmarkOperation.BlockSequence.SEQUENTIAL);
+            orderComboBox.addItem(BenchmarkOperation.BlockSequence.RANDOM);
+        }
+


@vpstackhub are you using vs code or netbeans for code editing? want to warn you that the entire constructor appears to have had it's indentation bumped by four making the entire function appear as a new change and the old remove. usually we want to avoid this so a git diff can in the review tell us specifically which lines changed instead of saying the entire function changed and the user has to look for the changes specifically. it might be good to use vs code to review the changes in the source control panel before making the commit so we an avoid accidental white space adjustments.

jamesmarkchan · 2025-10-07T05:20:44Z

src/jdiskmark/MainFrame.java

    public void initializeComboSettings() {
        typeCombo.setSelectedItem(App.benchmarkType);
-        loadSettings();
+        loadSettings();      


minor but looks like we are adding extra spaces after the line.

#78 - Implement Render Mode Options (Charts, Table, Charts + Table)

cae8c61

jamesmarkchan requested changes Sep 17, 2025

View reviewed changes

#78 - Final implementation: render mode options + shrunk banner

14f2da8

jamesmarkchan reviewed Sep 24, 2025

View reviewed changes

#78 - implement RenderFrequencyMode logic cleanup in BenchmarkWorker …

d8b0cb1

…and MainFrame

vpstackhub added 2 commits October 3, 2025 14:27

Rename Render Mode label to Display Mode

9c16e84

#78 - Added RenderFrequencyMode persistence and UI display in Benchma…

68c46bc

…rk Panel

jamesmarkchan requested changes Oct 7, 2025

View reviewed changes

jamesmarkchan changed the title ~~#78 - Implement Render Mode Options (Charts, Table, Charts + Table)~~ #78 - Implement Render Mode Options Oct 11, 2025

#78 - Implement Render Mode Options #89

Are you sure you want to change the base?

#78 - Implement Render Mode Options #89

Uh oh!

Conversation

vpstackhub commented Sep 17, 2025

Uh oh!

vpstackhub commented Sep 17, 2025

Uh oh!

jamesmarkchan commented Sep 17, 2025

Uh oh!

jamesmarkchan commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesmarkchan commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesmarkchan commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vpstackhub commented Sep 18, 2025 via email

Uh oh!

jamesmarkchan commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesmarkchan commented Sep 24, 2025

Uh oh!

vpstackhub commented Sep 30, 2025

Features Implemented

Code Changes

Testing Performed

Uh oh!

jamesmarkchan commented Sep 30, 2025

Uh oh!

vpstackhub commented Oct 3, 2025

Uh oh!

jamesmarkchan commented Oct 7, 2025

Uh oh!

jamesmarkchan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamesmarkchan commented Sep 17, 2025 •

edited

Loading

jamesmarkchan commented Sep 17, 2025 •

edited

Loading

jamesmarkchan commented Sep 19, 2025 •

edited

Loading