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

[RFC] Continuous zoom #175

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

mindhells
Copy link
Member

Please see #80 to better understand what we are trying to do here.

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

Merging #175 (ef216b8) into master (aab8a8f) will decrease coverage by 0.00%.
The diff coverage is 89.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #175      +/-   ##
==========================================
- Coverage   85.92%   85.91%   -0.01%     
==========================================
  Files          55       55              
  Lines       10389    10475      +86     
==========================================
+ Hits         8927     9000      +73     
- Misses       1462     1475      +13     
Impacted Files Coverage Δ
fract4d/fractconfig.py 93.13% <ø> (ø)
fract4d/messages.py 84.61% <50.00%> (-3.85%) ⬇️
fract4dgui/gtkfractal.py 83.88% <90.00%> (+0.69%) ⬆️
fract4dgui/preferences.py 77.30% <91.66%> (+0.69%) ⬆️
fract4d/fractal.py 91.94% <100.00%> (+0.03%) ⬆️
fract4dgui/main_window.py 63.96% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aab8a8f...ef216b8. Read the comment docs.

@edyoung
Copy link
Member

edyoung commented Dec 16, 2020

Hi folks! Just wanted to say - thanks a ton for all the work that has gone into this! Sorry I have been scarce the last few months. I've been super busy at work. I have some time off over the holidays so will aim to review, merge and release this as soon as we can.

@mindhells
Copy link
Member Author

Hi folks! Just wanted to say - thanks a ton for all the work that has gone into this! Sorry I have been scarce the last few months. I've been super busy at work. I have some time off over the holidays so will aim to review, merge and release this as soon as we can.

That's great news. I'm going to post the last updates in the related issue so you have all the details.
There's a couple of more things to add to this PR before it's ready for review, the idea is to have them before christmas.

`Location` (`X`, `Y` ... and `Z`, `W` in 4D space) and `Size` parameters will determine which point of the complex plane correspond to every pixel of the image (we leave out from this explanation the plane rotation parameters). This makes very likely to happen that subsequent frames, where location and size would change slightly, have pixels with the same, or very close point in the complex plane. So it makes sense to, instead of calculate the color of that pixel again (which is the heaviest process) we take the already calculated value.

The process to find those pixels with same or close complex plane point values, in an efficient way, is based in the assumption that _imaginary values `y` remain constant while moving across X axis, while real values `x` do the same for the Y axis_. And it consist in the following steps:
- find all the `x` values or columns for the new fractal (image) that are close enough to the previous image ones.
Copy link
Member

Choose a reason for hiding this comment

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

One way of thinking about this: imagine you are taking the previous frame and scaling it up. In frame 1, to get the color for pixel (0,0) we calculated point (x,y,z,w). In Frame 2, point (x,y,z,w) is now at a different pixel location, but if that location is still on screen, we can reuse the calculation and color pixel(a,b) with that point. I'm pretty sure that doesn't really depend on only x changing as we move across the screen or y changing vertically. We have a vector (dx,dy,dz,dw) which is the amount the point changes when we step 1 pixel to the left - we can use that to generalize this to work for rotated images or ones in other planes.

Copy link
Member Author

@mindhells mindhells Dec 21, 2020

Choose a reason for hiding this comment

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

That was exactly my thought when I first read https://github.com/xaos-project/XaoS/wiki/Developer's-Guide#saving-previous-pixels.
Then I said to myself "ok, let's try only XY plane (no plane rotation)". I was surprised when I found out you can actually change plane rotations and it keeps working. Except in certain cases.
As the Gnofract 4D manual states:

It allows you to treat a fractal which has more than one parameter as a four-dimensional object and interactively view slices of this object from arbitrary angles, giving rise to some very unusual images.

It seems that, as long as the slice you are viewing is parallel to one of the axis X or Y, the assumption is valid. In other words, if you rotate the slice respect to X and Y at the same time, then it doesn't work.
I tried to solve this issue/limitation, being playing around with the rotation matrix, point vectors... but honestly I don't have the math background to.

@mindhells mindhells marked this pull request as ready for review December 22, 2020 14:16
@mindhells
Copy link
Member Author

@edyoung is it ok if we merge this PR?
We haven't received any additional feedback so far and it would be a shame to leave this here forever. Besides, this feature could be enabled/disabled by configuration.

@edyoung
Copy link
Member

edyoung commented Apr 8, 2021 via email

@da2ce7
Copy link
Contributor

da2ce7 commented Apr 8, 2021

Needs Rebase.

fract4dgui/gtkfractal.py Outdated Show resolved Hide resolved
fract4dgui/preferences.py Outdated Show resolved Hide resolved
fract4dgui/gtkfractal.py Outdated Show resolved Hide resolved
@josebailo
Copy link
Contributor

Rebase done!

@cjmayo I removed the commented line and refactored the create_continuous_zoom_widget.

Regarding the flake8 check, I haven't found anything about using flake8 in the project so I'm not sure how to proceed. Are you using it by yourself or is established for developing?

@mindhells
Copy link
Member Author

Hi @edyoung ,
after rebase, test are failing because this condition is true:

if (m_thread.get_id() == std::this_thread::get_id())
{
// something is very wrong
std::cerr << this << "Waiting on own thread in IFractalSite::wait()!" << m_thread.get_id() << "\n";
throw std::exception();
}

can you elaborate under what circumstances this may happen? Maybe we're missing something anywhere else

@cjmayo
Copy link
Collaborator

cjmayo commented Apr 12, 2021

Regarding the flake8 check, I haven't found anything about using flake8 in the project so I'm not sure how to proceed. Are you using it by yourself or is established for developing?

It's only me. I just mentioned it because the single blank line before the class was obvious, that's fixed after the rebase.

@josebailo
Copy link
Contributor

josebailo commented Apr 28, 2021

Sorry for the delay.

I've been checking why the tests fails. First I thought this was because of some change on this branch, but I found a commit by @edyoung in another branch skipping this test because it fails intermittently.

I've skipped the test and it works now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants