Graph rework #370

felix-salfelder · 2015-09-19T15:24:28Z

continuing #314.... rebased

@in3otd i did not manage to disable qDebug per-file. i cleaned up some unnecessary output, does this improve the situation on your end?

storing indeps only. move dep into VarDep during schematic read-in. with this, VarPos.size() equals numAxes.

the dashed lines used to introduce unpredictable sizes. now Qt takes care of dashes.

a list of graphs is not a graph in the implied sense. hence the class that holds lists of graphs must be renamed. this makes room for a class dedicated to graphs, replacing the previous "Graph" class. a Graph object is now an iterable that yields sampling points. - rename Graph class to GraphDeque - implement new (iterable) Graph class dedicated to graphs

ScrPoints is supposed to hold the data required for plotting. previously, the ScrPoints were screen coordinates with an implicit correspondence to dat ('qucsdata') file contents.

Diagram (Painter, Marker) shall operate with the data held by these Graphs.

dissect parts of Marker::createText for later (re)use.

a marker position is a reference to a graph and a reference to a sampling point within this graph. currently, the graphs are one dimensional. change getSelected to return iterator, not int

move code from Marker::initText into GraphDeque::samplePos.

just assign Text, do not recompute coordinates. reuse code from Marker::initText put into GraphDeque::samplePos.

i'm wondering how this ever worked...

felix-salfelder · 2015-09-29T10:58:32Z

rebased ... (minor conflict)

in3otd · 2015-09-29T16:23:40Z

uh, oh, I get a qucs: graph.cpp:464: void Graph::ScrPt::setIndep(double): Assertion ScrX>=0 failed. with some graph here... did we already saw this in the past (cannot remember for sure) ??
Can try to put together a small failing example if you need (current one is quite big)

felix-salfelder · 2015-09-29T16:33:47Z

@in3otd i hope this is about a 3d plot. otherwise, an examples will likely be helpful.

the assertion is probably too harsh, in some intermediate stages it might be legal to insert negative coordinates. this makes sure that the code cannot be debugged easily. still a lot of guesswork involved here :(.

in3otd · 2015-09-29T18:46:29Z

no, it's a plain 2D plot.
With your latest commit, I get another assertion fail, qucs: graph.cpp:471: void Graph::ScrPt::setDep(cplx_t): Assertion ScrX>=0 failed

Here is a Qucs package with a small example (remove the .docx extension, GitHub needed that...)
Unpack, open the Data Display and follow the instructions there.

z_tmp_plot.qucs.docx

felix-salfelder · 2015-09-29T18:54:55Z

thank you.

just to be sure: what does "Unpack" mean?

$ file z_tmp_plot.qucs 
z_tmp_plot.qucs: data

my qucs open dielog does not list .qucs in the extensions dropdown.

in3otd · 2015-09-29T19:16:35Z

...sloppy description, I meant "Extract Package", from the GUI, Project->Extract Package (and no, I'm not sure to have seen this feature described in the docs...)

felix-salfelder · 2015-10-01T08:26:37Z

outch. functionally this appears to be a plain file archive. the extract thing created a directory and some files in $HOME/.qucs. shouldn't we switch to a more common format?! okay that was off topic.

@in3otd yes, i could reproduce the problem. see the latest commit for a workaround. let me think about how to fix this properly.

the warnings and asserts will not rescue us. remove them. this does not fix the magic number collision bug. currently points with negative screen coordinates are used as control tokens. need to get rid of that.

it does not work. must be implemented as part of the drawing routine.

guitorri · 2017-10-10T21:38:45Z

Thank you. I was about to ask you to rebase (or merge forward).
I read the changes. Seems fine, but I did not run yet.

felix-salfelder · 2017-10-10T21:40:25Z

i merged, because the rebase turned out to be messy. will rebase/squash when reviewed.

guitorri · 2017-10-10T21:50:19Z

I know, I tried to rebase myself... went ok till the 7/15 conflicts.

guitorri · 2017-10-13T18:28:05Z

Something is not right.

develop:

graph_rework:

Another example.
develop:

graph_rework:

I just loaded an exiting schematic. The Markers are gone and some Diagrams are painted elsewhere. I mean, if i scroll I see some outlines of diagrams moving around as if the Diagrams are being moved relative to the mouse cursor position.
It is algo giving a the following on the console Warning: QPainter::end: Painter ended with 5 saved states
Diagram axes are missing and sometimes the traces are plotted outside.

felix-salfelder · 2017-10-13T18:59:24Z

must be a side effect of the rebase. will have a look. thanks for the test.

:|

andresmmera · 2017-12-07T16:11:58Z

Hello,

I'm taking a look at this and I think that we should go back to ba8c29b and repeat the rebase against develop. I've already done that (several times...) but the branches have diverged a lot, so it is rather difficult to merge them properly...

Now I clearly see that the phasor and waveac diagrams need to be removed. Despite the fact that I honestly appreciate the effort of @gildias, these additions entangle (even more) the qucs code :-S

andresmmera · 2017-12-07T16:59:31Z

My intention is to create a branch removing #682 and then try to rebase this PR against that branch... WIP...

felix-salfelder · 2017-12-07T19:52:56Z

thanks a lot Andres.

andresmmera · 2017-12-08T09:21:10Z

Hello,

I created a branch (GRAPH_REWORK_wo_682) from ba8c29b which contains the latest commits from @felix-salfelder before merging "graph_rework" into develop. Then, as I said above, I created a branch (Remove682) identical to develop, but removing (manually) the features from #682

Then I rebased "GRAPH_REWORK_wo_682" against "Remove682", made some fixes to make it compile and tested the final result. It works and I didn't find bugs related to the merging process. However, I found a bug coming from ba8c29b (or even before). Please take a look at this, I guess this is related to the macro in marker.cpp, but I need to take a closer look.

At the light of these results, I think we should follow these steps:

Test "Remove682", open the corresponding PR and merge it into develop
In "GRAPH_REWORK_wo_682", fix the segfault I pointed out before, do more testing, open a PR and merge that into develop

And I think that's all 😄
@felix-salfelder @in3otd @ra3xdh @guitorri Do you agree?

felix-salfelder · 2017-12-09T13:11:51Z

wow. this looks promising.

about Remove682... maybe it makes sense to keep the code in some form, so it will be easier to add back later. e.g. move all the code to some diagrams/*.cpp, with some comments, and just remove these from the makefiles.

ba8c29b: will have a look!

andresmmera · 2017-12-09T15:21:27Z

e.g. move all the code to some diagrams/*.cpp, with some comments, and just remove these from the makefiles.

I agree to keep waveac.h/cpp and phasordiagram.h/cpp and remove them from the building system.
However, IMHO the other code related to #682 (I mean, those pieces of code that affect other files: diagram.cpp, etc.) could be removed since we'll need to figure out an alternative (non-intrusive) way to implement those diagrams.

guitorri · 2018-02-05T21:23:22Z

@andresmmera the first PR is merged.

What about https://github.com/andresmmera/qucs/tree/GRAPH_REWORK_wo_PR682
Does it need rebase or just a PR?

Please prepare the PR and we hunt the crash/bug later on.

andresmmera · 2018-02-06T18:16:27Z

It needs a rebase. I'm working on that, but it might take me a few days to have it done.

guitorri · 2018-07-25T19:55:35Z

Moving to next milestone.

felix-salfelder mentioned this pull request Sep 19, 2015

rework Graph and Marker #314

Closed

felix-salfelder added 6 commits September 29, 2015 12:55

VarPos size fixup

9400ac6

storing indeps only. move dep into VarDep during schematic read-in. with this, VarPos.size() equals numAxes.

dont fit memory size. just assert.

1d51b9e

the dashed lines used to introduce unpredictable sizes. now Qt takes care of dashes.

attach data to ScrPoints

b6c692e

ScrPoints is supposed to hold the data required for plotting. previously, the ScrPoints were screen coordinates with an implicit correspondence to dat ('qucsdata') file contents.

populate GraphDeque from Diagram::calcData

6255a3e

Diagram (Painter, Marker) shall operate with the data held by these Graphs.

Marker::assignText

14d5377

dissect parts of Marker::createText for later (re)use.

felix-salfelder force-pushed the graph_rework branch from 1cb07bb to 1c4b9a6 Compare September 29, 2015 10:55

felix-salfelder added 6 commits September 29, 2015 12:55

introduce explicit marker position

43061d5

a marker position is a reference to a graph and a reference to a sampling point within this graph. currently, the graphs are one dimensional. change getSelected to return iterator, not int

compute marker position in Marker::initText

04e01c8

move code from Marker::initText into GraphDeque::samplePos.

use marker position in moveLeftRight, moveUpDown

bd6867d

just assign Text, do not recompute coordinates. reuse code from Marker::initText put into GraphDeque::samplePos.

Use QVector (instead of std::vector) in an attempt to please travis.

c3b41d0

the build failed, adding SUFFIXES.

029d02f

i'm wondering how this ever worked...

removing some irrelevant qDebug() calls.

1c4b9a6

felix-salfelder changed the title ~~Graph rework~~ Graph rework (WIP) Sep 29, 2015

debugging

acddc6b

felix-salfelder mentioned this pull request Sep 29, 2015

3D cartesian plot is broken #386

Closed

felix-salfelder force-pushed the graph_rework branch from 92e99c5 to 6cf195f Compare October 1, 2015 08:20

negative screen coordinates...

9ce14d7

the warnings and asserts will not rescue us. remove them. this does not fix the magic number collision bug. currently points with negative screen coordinates are used as control tokens. need to get rid of that.

felix-salfelder force-pushed the graph_rework branch from 6cf195f to ba8c29b Compare October 6, 2015 07:52

disable hideLines in 3dplot.

ba8c29b

it does not work. must be implemented as part of the drawing routine.

in3otd mentioned this pull request Dec 17, 2016

Error messages in "graph.cpp" slow down debugging #632

Closed

guitorri changed the base branch from master to develop February 18, 2017 13:07

felix-salfelder mentioned this pull request Feb 18, 2017

Enhanced markers #407

Closed

felix-salfelder added this to the 0.0.20 milestone May 7, 2017

Merge branch 'develop' into graph_rework

08f3daa

felix-salfelder added 2 commits October 11, 2017 13:48

cleanup, try to make it compile

c3455fc

fix line continuation in a Makefile

9cf48dd

felix-salfelder changed the title ~~Graph rework (WIP)~~ Graph rework Oct 13, 2017

felix-salfelder mentioned this pull request Oct 14, 2017

fix Diagram::PaintDiagram visibility #739

Closed

felix-salfelder mentioned this pull request Dec 2, 2017

Correct marker misbehaviour when pointing to data outside the diagram limits #755

Closed

felix-salfelder requested review from andresmmera and removed request for andresmmera December 6, 2017 09:01

andresmmera mentioned this pull request Dec 16, 2017

Lay aside phasor and waveac diagrams #760

Merged

andresmmera mentioned this pull request Feb 8, 2018

Graph rework rebased #774

Closed

guitorri modified the milestones: 0.0.20, 0.0.21 Jul 25, 2018

felix-salfelder deleted the branch Qucs:develop July 2, 2024 08:22

felix-salfelder closed this Jul 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graph rework #370

Graph rework #370

felix-salfelder commented Sep 19, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Oct 1, 2015

guitorri commented Oct 10, 2017

felix-salfelder commented Oct 10, 2017

guitorri commented Oct 10, 2017

guitorri commented Oct 13, 2017

felix-salfelder commented Oct 13, 2017

andresmmera commented Dec 7, 2017

andresmmera commented Dec 7, 2017

felix-salfelder commented Dec 7, 2017

andresmmera commented Dec 8, 2017

felix-salfelder commented Dec 9, 2017

andresmmera commented Dec 9, 2017

guitorri commented Feb 5, 2018

andresmmera commented Feb 6, 2018

guitorri commented Jul 25, 2018

Graph rework #370

Graph rework #370

Conversation

felix-salfelder commented Sep 19, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Sep 29, 2015

in3otd commented Sep 29, 2015

felix-salfelder commented Oct 1, 2015

guitorri commented Oct 10, 2017

felix-salfelder commented Oct 10, 2017

guitorri commented Oct 10, 2017

guitorri commented Oct 13, 2017

felix-salfelder commented Oct 13, 2017

andresmmera commented Dec 7, 2017

andresmmera commented Dec 7, 2017

felix-salfelder commented Dec 7, 2017

andresmmera commented Dec 8, 2017

felix-salfelder commented Dec 9, 2017

andresmmera commented Dec 9, 2017

guitorri commented Feb 5, 2018

andresmmera commented Feb 6, 2018

guitorri commented Jul 25, 2018