weixin_39746241
2020-12-28 15:28 阅读 0

Warp Optimization

Some current hackery with optimization of Warp node. Will likely need some cleanup in morning, but both seem like pretty good additions.

The last commit can't be merged in this simple state: I just completely broke the ability for engines to depend on the current channelName. I'm thinking of adding a virtual member Warp::engineDependsOnChannelName, which can default true, but subclasses such as VectorWarp can set false to indicate that the current channel name doesn't affect them. Computing the sample regions repeatedly for different channels may not be that expensive compared to the actual sampling, but this helps cut down on the hashing as well. I just need that pr-speculative label.

该提问来源于开源项目:GafferHQ/gaffer

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享

8条回答 默认 最新

  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    Hey, does Warp::engineDependsOnChannelName sound OK to you?

    点赞 评论 复制链接分享
  • weixin_39552317 weixin_39552317 2020-12-28 15:28

    Do we actually have engines that depend on the channel name, or do you have some in mind for the future? If not, I'm almost tempted to say we should remove the possibility entirely. I did write a chromatic aberration type thing in the past that was a bit like a different warp in each channel, but the filtering was different to just a standard warp. If we do want to keep the possibility, maybe Warp::channelNameAffectsEngine() as a slightly less verbose alternative?

    点赞 评论 复制链接分享
  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    Do we actually have engines that depend on the channel name,

    No

    do you have some in mind for the future?

    Chromatic aberration is the only one I can think of

    I did write a chromatic aberration type thing in the past that was a bit like a different warp in each channel, but the filtering was different to just a standard warp.

    I don't see any strong reason why the filtering should be different? Chromatic aberration seems like it should just be a warp that is different for each channel.

    Talking to Lucien, it seems like we should keep the possibility of chromatic aberration open, but he also points out that you could do this with 3 Warps and some CopyChannels. There's probably no real reason not to do it that way, so I'm fine with ripping the channelName out of the API.

    点赞 评论 复制链接分享
  • weixin_39552317 weixin_39552317 2020-12-28 15:28

    I don't see any strong reason why the filtering should be different? Chromatic aberration seems like it should just be a warp that is different for each channel.

    For large aberrations, I was basically doing a linear blur in the direction of the warp to produce a soft spectrum rather than a series of hard edges where each channel was warped differently. If the separation is much more than a pixel or two it becomes pretty ugly otherwise.

    点赞 评论 复制链接分享
  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    Right, that makes sense.

    If we wanted what the current API provides, we could just use 3 warps and some CopyChannels, if we wanted something higher quality like that, we would need something else anyways. Sounds like a solid argument for doing away with the channel in the current API for performance reasons.

    点赞 评论 复制链接分享
  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    OK, I've updated this PR to now just remove channelName from the computeEngine API, as suggested.

    点赞 评论 复制链接分享
  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    Addressed 3/4 comments from Andrew. The 4th I will argue with him on Monday ( I prefer not setting something to NULL when it's about to be unconditionally assigned to anyway, but it doesn't matter too much and I could certainly be persuaded ).

    Given that I haven't gotten far into the context optimization stuff yet, I would kind of like eyes on from John before I merge this, given that I'm introducing sampleRegionsEmptyTile() which feels a bit new ( though there is precedent in ImagePlug::blackTile() ), and I'm changing the computeEngine API ( which John suggested, but hasn't looked at yet ). I'll see how I'm feeling Monday.

    点赞 评论 复制链接分享
  • weixin_39746241 weixin_39746241 2020-12-28 15:28

    In order to keep forward momentum, Andrew is in favour of going ahead and merging this so I can work on top of it. Might still be good for John to take a look through it tomorrow.

    点赞 评论 复制链接分享

相关推荐