Mirrored Input/Output Graph Mode

[BenBE]

This extends the existing graph mode to allow for vertically mirrors graphs representing input and output rates.

This basically implements the idea from #1390 based on the #1387 that enables only specific meters to implement these additional modes.

@Explorer09 This is what I meant with code re-use … Most of the added code is from additional constants, with most of the existing code re-used with only minor changes.

NB: There's some known bugs as I needed to slightly change the memory layout for the Meter->values data storage for the DiskIO/NetworkIO meters, which causes issues in bar meter mode.

[Explorer09]

@Explorer09 This is what I meant with code re-use … Most of the added code is from additional constants, with most of the existing code re-used with only minor changes.

I'm not a fan of this way of changing code. Expect for the addition of a new meter mode, I have different opinions on how the code can be done.

Firstly:

There's some known bugs as I needed to slightly change the memory layout for the Meter->values data storage for the DiskIO/NetworkIO meters, which causes issues in bar meter mode.

We're now displaying two different kinds of data for the DiskIO meter. I suggest splitting the meter into two. The meter named DiskIO keeps the current behavior of displaying the utilization percentage, and the other, which I would name DiskIOSpeed for now, shows the read and write speeds. This way we don't have to use different modes to display different kinds of data.

For NetworkIO meter, there's no need to split.

Second. I don't like the GraphMeterMode_drawGraph the way you implemented it.

https://github.com/htop-dev/htop/pull/1701/files#diff-9ea32cfb9d3bca3e912da3efac9082542fb87f3689561174d2e18bf33a07dae3R353-R358

A few issues: (a) it works with GRAPH_HEIGHT that is even only (my #1415 code did address the case where the height is odd); (b) the new channel argument for the function is a control coupling that I wish to avoid.

My vision was the GraphMeterMode_draw function be used for both meter modes (the original Graph and the new mode that you called IO), and within the draw function the meter's mode property is read, which determines how the dots should be drawn. As I have written #714, I considered that the new feature code might be more maintainable if I base #1415 on #714. And that was how it went.

And there are two more minor things:

• Do you consider cherry-picking this commit (Use "h" property as height when drawing Graph meter) in #714?
• I'm not sure IO_METERMODE is a good name. I think the name of the new mode is worth discussing. (I would simply call the mode Graph2 even though you don't need to take my advice)

[BenBE]

No objection on cherry-picking https://github.com/htop-dev/htop/pull/714/commits/c7c0cdad60f60a2d04e477a52ebc5400a8165a84 … That one can go right in AFAICS. Though I had to do some minor adjustments.

[BenBE]

Regarding the name, I'm not particular focused on the name and looking at what these kinds of graphs are commonly referred to Bipolar sounds like a viable option/alternative to me.

Regarding the approach: My approach focuses on re-using as much of the existing code as possible, adding small parameters for where different behaviour is required. My approach also avoids quite a large chunk of "derive some UTF-8 sequence" code you use for determining the character to place on the graph.

Splitting the DiskIO meter into one for utilization and another for bandwidth might be an option; but as you see in my PR: That's just a matter of updating these meters to either support this graph style or not.

The major issue I really see with #714 and thus also #1415 is the very huge upfront code change that introduces quite complex code. OTOH this PR comes with very little up-front changes and even the changes it makes are easy to understand. The time it took me to write this PR is roughly the same it took me to go through #714's dot placing function; and while I'm kinda confident with the code in this PR I won't really say the same for #714.

[Explorer09]

@BenBE Did you consider incorporating the "dynamic scaling" part of #714? I think that part would be needed for your changes here. Specifically these:

596fa20d Introduce isPercentChart member in MeterClass 0995f49b Update "total" value for non-percent bar meters e237eed7 Graph meter dynamic scaling and percent graph drawing f7fee7fd Adjust LoadAverageMeter "total" assignment 42c111ef TasksMeter: remove code for auto-updating "total" 7ae110b8 NetworkIOMeter: remove code for auto-updating "total"

I'm not sure if I need to file a separate PR for that. (Update: Please incorporate #1721 instead.)

The reason is something to do with the "total" value: The DiskIO and NetworkIO meters you are working on do not have a maximum of the read and write speeds. With the above patches, you can have "dynamic total" values that work consistently across different kinds of meters.