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

3D cartesian plot is broken #386

Closed
ra3xdh opened this issue Sep 29, 2015 · 32 comments
Closed

3D cartesian plot is broken #386

ra3xdh opened this issue Sep 29, 2015 · 32 comments

Comments

@ra3xdh
Copy link
Contributor

ra3xdh commented Sep 29, 2015

I have just found another critical bug in Qucs-GUI. 3D-cartesian plot is broken in master. Open two schamtics with 3D plots in 0.0.18 and 0.0.19 and compare results to reproduce this bug. 3D plot in 0.0.19 looks unreadable. Here is example screenshot. Left is Qucs-0.0.18, right is Qucs-0.0.19.
3d-diagr

@ra3xdh ra3xdh added this to the 0.0.19 milestone Sep 29, 2015
@felix-salfelder
Copy link
Member

it seems, the new code does not print as many cross connections.

the missing blue sample dots in the left picture are more interesting. is this related to the z buffer? what happens if you slightly turn the plot to the left (= move the camera to the right), will they reappear?

please post the data file and the schematic somewhere, i will have a look.

@ra3xdh
Copy link
Contributor Author

ra3xdh commented Sep 29, 2015

Here is test schematic https://gist.github.com/ra3xdh/eb92c81d7d72d401668b/raw/12be21db82ff579e91a9a7e4aa63370a25df2ba3/diode_dblswp_qucs.sch 3D cartesian plot is embedded in schematic.

what happens if you slightly turn the plot to the left (= move the camera to the right), will they reappear?

Yes, missing points could appear when plot is rotated at specific angle around Z-axis.

@felix-salfelder
Copy link
Member

thanks

whether the cross connections are painted or not depends on the angle. must be a bug in the zbuffer code. also, i get a lot of "dangerous: returning negative screen coordinate" messages, which may or may not be related...

@felix-salfelder
Copy link
Member

apparently, hideLines is broken for 3dplots. i am working on this in #370. previously the z buffer has been monkey-patched into the 2d ploting routines. these have been replaced by the QtPainter.

i think i will implement 3dgraphs with their own drawings...

@guitorri
Copy link
Member

guitorri commented Feb 1, 2016

If at a certain angle the user still can get a useful image, then I will count it as know issue/regression.
I will move this to the next release, I hope it is ok if you guys.

@guitorri guitorri modified the milestones: 0.0.20, 0.0.19 Feb 1, 2016
@felix-salfelder
Copy link
Member

i have failed to make the old code work again. wasted a lot of time on this :(

using qt for the 2d plots is working pretty well. i think we should head for something like qwtplot to do the 3d plots. unfortunately qwtplot does not support multiple layers (yet)...

@ldpgh
Copy link
Contributor

ldpgh commented Oct 29, 2016

Simple check with x=5 SweepPts : y=5 SweepPts

  • 3D chart placed, hide invisible lines = on, all other settings = default
  • The dataset is created (simulated) in 0.0.19 (0.0.9 has an equation issue) and just loaded to the other revisions.

Screenshots of the different revisions ... looks like an evolution

  • Qucs 0.0.19 ... just a few boundary points left ... Qt4 / 2016_3_20
    qucs_rect3ddiagram_0 0 19
  • Qucs 0.0.17 - 0.0.18 ... visible line segments missing, at least one hidden line segmet visible ... Qt4 / 2013_6_23 - 2014_8_31
    qucs_rect3ddiagram_0 0 18
  • Qucs 0.0.11 - 0.0.16 (2011) ... some visible line segments missing ... Qt3 / 2007_3_17 - 2011_3_17
    qucs_rect3ddiagram_0 0 16
  • Qucs 0.0.10 ... few visible line segments missing ... Qt3 / 2006_8_31
    qucs_rect3ddiagram_0 0 10
  • Qucs 0.0.9 (2006) ... seems to be OK ... Qt3 / 2006_5_27
    qucs_rect3ddiagram_0 0 9

@ldpgh
Copy link
Contributor

ldpgh commented Oct 29, 2016

Simple check with x= 3 ... 5 SweepPts : y=5 SweepPts

  • 3D chart placed, hide invisible lines = on, all other settings = default
  • It seems the issue has a strong correlation to the point count of the x-Axis (1st Sweep)
  • It seems the issue has no correlation to the point count of the y-Axis (2nd Sweep)

Screenshots using 0.0.19 and different point size of the x-Sweep

  • 2pts ... makes less sense ... it's simply flat
  • 3 pts ... already one strange line segment @ xyz=0V/5V/0V
    qucs_rect3ddiagram_0 0 19__3pts
  • 4 pts ... hidden line segment not hidden @ xyz=0V/2V/0V
    qucs_rect3ddiagram_0 0 19__4pts
  • 5pts ... same as the screenshot in the version related comment above ... most segments gone
    qucs_rect3ddiagram_0 0 19__5pts

@ldpgh
Copy link
Contributor

ldpgh commented Oct 29, 2016

Is there any Unit-test associated to rect3ddiagram.cpp?

@felix-salfelder
Copy link
Member

i remember we had been discussing unit tests for the GUI, something about graphical diffs and they are somewhere on a whishlist.
@ldpgh do you know how to implement that? please don't hesitate!

@ldpgh
Copy link
Contributor

ldpgh commented Nov 1, 2016

Environment: Win7, qucs-HEAD, CMake in QtCreator 3.1.2 @ Qt4.8.6, mingw482.

There is a significant difference of appearance between debug-mode (significant more line segments visible in 0.0.19, regardsless breakpoints) vs. non-debug-mode.

Have you experienced similar behavior?

@felix-salfelder
Copy link
Member

as you might have guessed, the debug mode must not interfere with the behaviour of the programme (except for printout to stderr), so that's another bug.

@in3otd
Copy link
Contributor

in3otd commented Nov 1, 2016

...interesting, maybe some floating point comparisons are giving different results, since the variables may end in registers instead of memory. I think we saw something similar in the simulator in the past.

Doing a quick diff of rect3ddiagram.cpp between version 0.0.6 and the current one doesn't appear to show any big changes (besides some slightly different rounding/truncation), at a first glance, but I may well have overlooked something.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

@in3otd ... current version vs version 0.0.6 ... Do you mean Qucs version 0.0.6, because for rect3ddiagram.cppit looks to me there are 2 major implementation changes:

  • some data types involved are change from intto float
  • from 1-dim pointer array (p, p+1) to pointer array with points (p.x, p.y) including change of method definitions.

In addition it seems some changes are caused by graph.cpp/.h respective lead to changes in graph.h/.cpp. It's not expect with rect3ddiagram beeing the only 3D diagram and the associated requirements.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

Not verified/debugged yet . . . just guessing (!!!) . . . will check it next days.
The differences smells like "malloc without memset" (e.g. removeHiddenLines()). It would explain some strange behavior related how the application is started respective the previous run.

What is the more appropriate direction . . . taking into accound all the side effects

  1. fix current revision with all the floats
  2. lift up the implementation of 0.0.9 (which sounds gracy) to match current 0.0.19 implementation of graph.h/.cpp and diagram.h/.cpp

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

Finding-1

(1) Start in Run-Mode (Ctrl-R) ...

(2) Load rect3ddiagram1x_test_sch.sch
qucs_rect3ddiagram_0 0 19__one_kind

(3.1) Load rect3ddiagram7x_test_sch.sch
(3.2) Close Load rect3ddiagram7x_test_sch.sch
(3.3) Close Load rect3ddiagram1x_test_sch.sch

(4) Load rect3ddiagram1x_test_sch.sch
qucs_rect3ddiagram_0 0 19__other_kind

Note-1: Closing order @ 3.2/3.3 has impact : 7x before 1x
Note-2: Hide/Unhide of SelectionWindow has impact too !

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

Finding-2

with hiddenenabled and return value of isHidden() overriden to true all drawn line segments look OK ... means looks the same as hidden=false.
This is surprising to me, because I have not just missing segments. Sometimes line segmets start/end outside the diagram-area, which would point to the framebuffer and associated files.
qucs_rect3ddiagram_0 0 19__outlier

@felix-salfelder
Copy link
Member

... also try to plot 3D data (i.e. sweep over 3 parameters). that gives a rough idea of what the code is trying to do. then 4.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

It's cool stuff (with hideLine=true) ... for this reason it make sense to spend some time with it.
qucs_rect3ddiagram_0 0 9__3waves

And a snapshot with 4 sweep for completeness.
qucs_rect3ddiagram_0 0 9__3waves_5sweeps

And a chart with all line segments visible for comparison.
qucs_rect3ddiagram_0 0 9__3waves_nohide

3 pictures added just to see the goal :-)

And a screenshot of the current status (0.0.19 @ Oct 2016) ...
qucs_rect3ddiagram_0 0 19__3waves

@ldpgh
Copy link
Contributor

ldpgh commented Nov 2, 2016

I really like to get a feedback regarding appropriate direction . . . taking into accound all the side effects

  1. fix current revision 0.0.19 with all the floats.
  2. lift up again the implementation of 0.0.9 (which is the other side of the medal) to match current 0.0.19 implementation of graph.h/.cpp and diagram.h/.cpp. It's unknow to me what get lost in this case.

It guides my debug strategy/procedure ... even the recommended one may not be the best solution.
Tried to handle both ... but keep going with both would require at least one more head on my shoulders ;-)

@felix-salfelder
Copy link
Member

the reason why i rewrote parts of the plotting (mostly unmerged work) is, that i would like to read data from other sources than just .dat files. think of hdf5 or a simulator as a possible alternative souce (for now). the dat format is absolutely unsuited for this type of task, but unfortunately, the plotting routines more or less work on a binary image that is layed out like these .dat files... which is where the shit is hitting the fan.

that said, when i tried to fix the 3d plot (yes, i'd better be able to, but i wasnt), it was pretty clear to me that this would the wrong track. it never worked right (and not only the comments indicate that it is trial-and-error-spaghetticode). one part of this desaster is, that the 3dplot and the 2dplot are heavily entangled, and they should not be. if you can untangle it, then yes, lift 0.0.9. but please only for the 3d plotting. if you prefer a sane path, please help with #370, it fixes some other problems with the markers as well...

please let me know, if you need more info. will have more time for this after the symbols are in place...

@ldpgh
Copy link
Contributor

ldpgh commented Nov 7, 2016

... continuing debug with HEAD (9d924ca)

Finding-3
rect3ddiagram.cpp : isHidden() + hideLines=true (!)

  • return 1==0;
  • all line segments are drawn. It seems graph.cpp/h & diagram.cpp/h are not involved respective do not modify what has to be drawn
  • isHidden(...) is called just by calcLine(...)

rect3ddiagram.cpp : removeHiddenLines() + hideLines=true (!)

  • free(zMem); return; @ line 496 ... all line segments are drawn
  • free(zMem); return; @ line 549 ... reduced output as already known (just few line segments are drawn)
  • it seems to confirm assumption regarding graph.cpp/h & diagram.cpp/h

rect3ddiagram.cpp : calcLine(...)

  • is called 1x by removeHiddenCross(...)
  • is called 4x by removeHiddenLines(...) ... within the lines 496-549

@ldpgh
Copy link
Contributor

ldpgh commented Nov 7, 2016

Finding-4
rect3ddiagram.cpp : calcCoordinate3D(...) + hideLines=true (!)

p->x  = int(calcX_2D(x, y, zr) + 0.5 + xorig);
p->y  = int(calcY_2D(x, y, zr) + 0.5 + yorig);

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . ^^^
Remove of 0.5 brings a couple of line segments back, but it seems to be just a sideeffect. Reopen of the schematic brings sometimes the old status back.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 8, 2016

Modifications just in rect3ddiagram.cpp improve the behavior ... but not fixed to 100%.

Screenshot of the 0.0.19Mod @ 13th Nov 2016
qucs_rect3ddiagram_0 0 19mod13nov__3waves

Screenshot of the 0.0.19 @ Oct 2016
qucs_rect3ddiagram_0 0 19__3waves

In addition there are a test schematics ... maybe qucs-test is a good place for it as long as rect3ddiagram is part of the game.

Would like to have someone else to check the modification ... who spends a little bit of time?

@ldpgh
Copy link
Contributor

ldpgh commented Nov 13, 2016

Finding-5
rect3ddiagram.cpp : removeHiddenLines(...) . . . segmentation fault

In order to chase the random behavior the initialmalloc() for zMem& Memhas been changed from
malloc( (Size+2)*sizeof(...)) -> malloc( 8*(Size+2)*sizeof(...)) ... just to prevent the realloc(). It seems as result of the extended allocated memory no more lines leaving the diagram area ... but it's hard to verify this assumption.

Adding test-case diode_dblswp_qucs.sch with 200pts @ SW1 has brought up a memory issue (segmentation fault at fprintf for debugging respective @ calcLine : pMem->x = ...) while turning the malloc back to the original settings (remove of 8*).

With the modified setting (8*(...)) diode_dblswp_qucs.sch runs with 20000pts @ SW1 and 50pts @ SW2 with no issues.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 13, 2016

0.0.19Mod @ 13th Nov 2016 ... modifications are in rect3ddiagram.cpp . The file is based on commit 9d924ca

Comparison ... 0.0.9 ... 0.0.19 ... 0.0.19 Mod @ 13th Nov 2016 ... and how it looks at my screen.
cmp_revs_small_000
cmp_revs_small_test
cmp_revs_test_sch
cmp_revs_7x
cmp_revs_dio
cmp_revs_waves3x_sweeps4x
cmp_revs_waves3x_test

@felix-salfelder
Copy link
Member

wow. care to open a PR with the modified recd3dthing.cpp?

@guitorri
Copy link
Member

@ldpgh can you share your fix? If you do we can include it already on 0.0.19 which is about to be released.

@ldpgh
Copy link
Contributor

ldpgh commented Nov 16, 2016

@ guitorri & Felix ... "share the files" is the plan.

Understand Git/Github PR-procedure is still a hurdle. And I'm still fiddling with my debug comments spread over the entire file ... to a large degree no longer valid.

Is there any deadline? My plan is to get it done until end of this week.

@guitorri
Copy link
Member

I tentatively put December 1st for the release. So take your time and let us know if you need any assistance. This might help: https://github.com/Qucs/qucs/wiki/Contribution. In particular the workflow section.

@felix-salfelder
Copy link
Member

debug comments spread over the entire file

git add -p might be useful. rescued me a couple of times (out of similar situations).

ldpgh pushed a commit to ldpgh/qucs that referenced this issue Dec 13, 2016
…to bring the hidden lines back. Test case are in qucs-test\testsuite\GUI_rect3ddiagram_prj
guitorri pushed a commit to guitorri/qucs that referenced this issue Jan 19, 2017
Modify rect3ddiagram.cpp to bring the hidden lines back.
Test cases are in qucs-test\testsuite\GUI_rect3ddiagram_prj.
guitorri added a commit that referenced this issue Jan 19, 2017
@guitorri guitorri modified the milestones: 0.0.19, 0.0.20 Jan 19, 2017
@guitorri
Copy link
Member

Fix merged into release-0.0.19. Thank you ldpgh.

ra3xdh pushed a commit to ra3xdh/qucs that referenced this issue Jan 29, 2017
Modify rect3ddiagram.cpp to bring the hidden lines back.
Test cases are in qucs-test\testsuite\GUI_rect3ddiagram_prj.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants