Update sun_position to Blender 2.82 #69936

New Issue

Damien Picard · 2019-09-16T11:33:26+02:00

Damien Picard commented

2019-09-16 11:33:26 +02:00

Hi, I see that @BrendonMurphy updated the sun_position add-on in blender/blender-addons-contrib@7b0831a775. I have been working on updating it as well for a while, and although the map feature is still missing, the rest of the add-on is done, and is much better integrated into Blender’s UI/UX.

Here is a list of some changes:

Move preferences to user preferences
Convert UI to flow, remove two-column mode
Remove activation operator. The add-on is now always on
- Update is done with prop update functions and a handler instead
Disabling options in preferences now have an effect on the scene
HDR selection is now an simpler operator. Click in 3D view to select sun position
Presets are now actual Blender presets, so the user can add their own
Object groups for analemma visualisation is now a collection
Cleanup and refactor
- Remove some manager classes and operators
- Style (some pep8, snake_case, variable names)

I rebased upon blender/blender-addons-contrib@7b0831a775, so the patch should apply cleanly. I won’t push this straight away as it’s not a small change.

sun_pos_update_2.8.patch

Hi, I see that @BrendonMurphy updated the sun_position add-on in blender/blender-addons-contrib@7b0831a775. I have been working on updating it as well for a while, and although the map feature is still missing, the rest of the add-on is done, and is much better integrated into Blender’s UI/UX. Here is a list of some changes: - Move preferences to user preferences - Convert UI to flow, remove two-column mode - Remove activation operator. The add-on is now always on - Update is done with prop update functions and a handler instead - Disabling options in preferences now have an effect on the scene - HDR selection is now an simpler operator. Click in 3D view to select sun position - Presets are now actual Blender presets, so the user can add their own - Object groups for analemma visualisation is now a collection - Cleanup and refactor - Remove some manager classes and operators - Style (some pep8, snake_case, variable names) ![image.png](https://archive.blender.org/developer/F7750775/image.png) I rebased upon blender/blender-addons-contrib@7b0831a775, so the patch should apply cleanly. I won’t push this straight away as it’s not a small change. [sun_pos_update_2.8.patch](https://archive.blender.org/developer/F7750724/sun_pos_update_2.8.patch)

Brendon Murphy was assigned by Damien Picard

2019-09-16 11:33:26 +02:00

Damien Picard commented

2019-09-16 11:33:26 +02:00

Added subscribers: @BrendonMurphy, @pioverfour

Brendon Murphy commented

2019-09-17 05:15:01 +02:00

hi, nice work. we can do this I'll check with you in blender.chat before go ahead. nBurn and I were discussing some issues a couple of days ago in irc, it will be worth a quick discuss before push.

Damien Picard commented

2019-09-17 12:25:34 +02:00

That’s fine with me. Some points which certainly need discussion are:

The reimplementation of the HDR and world map modes, which were in my opinion overly complex in the previous version, but maybe they fit others’ use cases. The shader part can definitely be simplified using the new gpu and gpu_extras modules, but I’m not sure about the UI.
- For the HDR sun selection, I don’t think the user needs to display it permanently, since the purpose is to choose a precise point on the texture and then sync it, so a one-time operation fits better.
- For the world map, I’m not sure overlaying it on the 3D View or properties panel is right. Maybe displaying it inside an image editor would be better, but I’m not sure yet.
The presets: I made a non-standard operator to copy defaults to the user directory, but I’m not sure it’s really useful for everybody (two Boston presets, two Honolulu presets…?).
The UTC zone’s behaviour is not clear to me, despite its description in the old wiki.

I have only tested the add-on under Linux, so there might be problems with graphics on other platforms.

Should I open a Diff to further review the code?

That’s fine with me. Some points which certainly need discussion are: - The reimplementation of the HDR and world map modes, which were in my opinion overly complex in the previous version, but maybe they fit others’ use cases. The shader part can definitely be simplified using the new `gpu` and `gpu_extras` modules, but I’m not sure about the UI. - For the HDR sun selection, I don’t think the user needs to display it permanently, since the purpose is to choose a precise point on the texture and then sync it, so a one-time operation fits better. - For the world map, I’m not sure overlaying it on the 3D View or properties panel is right. Maybe displaying it inside an image editor would be better, but I’m not sure yet. - The presets: I made a non-standard operator to copy defaults to the user directory, but I’m not sure it’s really useful for everybody (two Boston presets, two Honolulu presets…?). - The UTC zone’s behaviour is not clear to me, despite its description in the old wiki. I have only tested the add-on under Linux, so there might be problems with graphics on other platforms. Should I open a Diff to further review the code?

Damien Picard commented

2019-09-17 16:44:48 +02:00

I opened a diff at D5825, and restored mirrorball projections for HDRI sun selections.

I opened a diff at [D5825](https://archive.blender.org/developer/D5825), and restored mirrorball projections for HDRI sun selections.

Brendon Murphy commented

2019-09-17 23:41:07 +02:00

Added subscriber: @nBurn

Brendon Murphy commented

2019-09-17 23:41:07 +02:00

hi @pioverfour

The re implementation of the HDR and world map modes, which were in my opinion overly complex in the previous version, but maybe they fit others’ use cases. The shader part can definitely be simplified using the new gpu and gpu_extras modules, but I’m not sure about the UI.

I would not worry about the hrdi/world map just yet.
The hdri button can stay for now if it's working.
The World map, keep the current code intact, it's ok for now to leave it. There's several options on how to handle it,
Implement in old style.
Put in some kind of open/close show/hide panel.
Possibly there's code for custom editor types now, could even do that and pop it up?

We do need more thinking on that and it's not a priority atm. I think it's best to wait, I've asked newbs to look in a month when he's back from holiday. There's other bgl experts that may be able to help/provide answers yet to be contacted.

The presets become a little more important if there's no world map? I think at some later stage they could be improved.

Some history with this fix:
https://github.com/kevancress/sun_position_b28_test was the base for the update.
@nBurn had a look and made some changes, then I made some changes.
Now it's in your hands so to speak. I guess at one stage there were 3 of us working on it...

Your code has had basic look over, it looks ok, much cleaner than what we initially committed. Thanks! If you can could you zip upload the whole file here in this task. I want to look at it as a whole addon and I'll compare all the files, double check some issues I was looking at.
Thanks again.

hi @pioverfour > The re implementation of the HDR and world map modes, which were in my opinion overly complex in the previous version, but maybe they fit others’ use cases. The shader part can definitely be simplified using the new gpu and gpu_extras modules, but I’m not sure about the UI. - I would not worry about the hrdi/world map just yet. - The hdri button can stay for now if it's working. - The World map, keep the current code intact, it's ok for now to leave it. There's several options on how to handle it, - Implement in old style. - Put in some kind of open/close show/hide panel. - Possibly there's code for custom editor types now, could even do that and pop it up? We do need more thinking on that and it's not a priority atm. I think it's best to wait, I've asked newbs to look in a month when he's back from holiday. There's other bgl experts that may be able to help/provide answers yet to be contacted. - The presets become a little more important if there's no world map? I think at some later stage they could be improved. Some history with this fix: https://github.com/kevancress/sun_position_b28_test was the base for the update. @nBurn had a look and made some changes, then I made some changes. Now it's in your hands so to speak. I guess at one stage there were 3 of us working on it... Your code has had basic look over, it looks ok, much cleaner than what we initially committed. Thanks! If you can could you zip upload the whole file here in this task. I want to look at it as a whole addon and I'll compare all the files, double check some issues I was looking at. Thanks again.

Damien Picard commented

2019-09-18 11:00:43 +02:00

All right, I’ll give it a rest for the time being, the map can be reimplemented later. The current code for it won’t work at all since it relies on the Display manager class, which I removed. But it was broken for a long time anyway, I had to use Blender 2.75 to see how it behaved.

Thanks for the history, I didn’t know so much work had gone into this already…

Anyway, here is the zipped add-on.
190918_sun_position.zip

All right, I’ll give it a rest for the time being, the map can be reimplemented later. The current code for it won’t work at all since it relies on the `Display` manager class, which I removed. But it was broken for a long time anyway, I had to use Blender 2.75 to see how it behaved. Thanks for the history, I didn’t know so much work had gone into this already… Anyway, here is the zipped add-on. [190918_sun_position.zip](https://archive.blender.org/developer/F7755084/190918_sun_position.zip)

Damien Picard commented

2019-09-18 18:15:45 +02:00

Some fixes for the HDRI point selector, implemented geographic coordinates parser field using a GPL module by Maximilian Högner , to allow users to paste coordinates from an online map.
190918_sun_position.v01.zip

Some fixes for the HDRI point selector, implemented geographic coordinates parser field using [a GPL module by Maximilian Högner ](http://hoegners.de/Maxi/geo/), to allow users to paste coordinates from an online map. [190918_sun_position.v01.zip](https://archive.blender.org/developer/F7755933/190918_sun_position.v01.zip)

Brendon Murphy commented

2019-12-03 01:43:51 +01:00

hi, still some work to do here. After discussions we will remove the not working world map and restore some sane presets.
Small code clean up will follow and committed to contrib.
We will then look at viability of 2.82 release.

hi, still some work to do here. After discussions we will remove the not working world map and restore some sane presets. Small code clean up will follow and committed to contrib. We will then look at viability of 2.82 release.

Brendon Murphy changed title from ~~Update sun_position to Blender 2.8~~ to Update sun_position to Blender 2.82

2019-12-03 01:44:21 +01:00

Damien Picard commented

2019-12-04 12:10:24 +01:00

I proceeded with removing the world maps (blender/blender-addons-contrib@ab0c257), and replacing presets (blender/blender-addons-contrib@6eb516c). I also did various fixes and cleanups (blender/blender-addons-contrib@b7aae31, blender/blender-addons-contrib@bd4f0a0, blender/blender-addons-contrib@c74a4e7, blender/blender-addons-contrib@b52e776) and fixes to match the [online calculator ](https://www.esrl.noaa.gov/gmd/grad/solcalc/) upon which this add-on is based (blender/blender-addons-contrib@e79295c, blender/blender-addons-contrib@e8696b0).

Brendon Murphy commented

2019-12-09 02:13:27 +01:00

hi, thanks for all the requested changes. This is now ready to land.
Please tag this task in the 2 commits. 1st remove from addons contrib, 2nd commit to add to release addons.

hi, thanks for all the requested changes. This is now ready to land. Please tag this task in the 2 commits. 1st remove from addons contrib, 2nd commit to add to release addons.

Brendon Murphy commented

2019-12-09 22:43:31 +01:00

Changed status from 'Open' to: 'Resolved'

Brendon Murphy closed this issue

2019-12-09 22:43:31 +01:00

Brendon Murphy commented

2019-12-09 22:43:31 +01:00

committed and closing. Thanks @pioverfour for the time and effort put into this.

Damien Picard referenced this issue from a commit

2023-11-17 14:13:37 +01:00

Sun Position: remove presets

Sign in to join this conversation.